Keep deleting lines when triggering deleteAllLeft on column 1 #28392
Conversation
alfondotnet
force-pushed
the
feature/cmd-delete
branch
from
f5f2930 to
0a30af0
Compare
alfondotnet
force-pushed
the
feature/cmd-delete
branch
3 times, most recently
from
83e42bc to
cf5b2a8
Compare
|
@rebornix any concerns with this change? thanks! |
|
@alfonsoperez I really like the idea and the code looks good. The only thing we forget to take into consideration is multi cursor. Let's say we have below content
Note the cursors are before the last character of each line. Then we press cmd+backspace
Everything looks pretty good. Now we have a problem with what's the expected result of another cmd+backspace. Do you think it should behave the same as a simple backspace backspace
cmd+backspace The command is called delete all left. While in this case, the cursor is at the beginning of the line, there is nothing on the left, so I would expect another Delete All Left will just delete the line break before the cursor. While right now, the result is correct but the result selections are not the same as a backspace. What do you think on this? |
|
BTW, if you play with Sublime Text, you can see that cmd+backspace works the same as backspace in this particular case. Our |
|
Good point!, thanks for the detailed review!, I will take a look |
alfondotnet
force-pushed
the
feature/cmd-delete
branch
from
cf5b2a8 to
87c413f
Compare
|
|
build/dependencies.js
Outdated
| @@ -43,7 +43,7 @@ function asYarnDependency(prefix, tree) { | |||
| } | |||
|
|
|||
| function getYarnProductionDependencies(cwd) { | |||
| const raw = cp.execSync('yarn list --json', { cwd, encoding: 'utf8', env: { ...process.env, NODE_ENV: 'production' }, stdio: [null, null, 'ignore'] }); | |||
| const raw = cp.execSync('yarn list --json', { cwd, encoding: 'utf8', env: Object.assign({}, process.env, { NODE_ENV: 'production' }), stdio: [null, null, 'ignore'] }); | |||
There was a problem hiding this comment.
my project didn't build with this spread here
| let edits: IIdentifiedSingleEditOperation[] = effectiveRanges.map(range => { | ||
| endCursorState.push(new Selection(range.startLineNumber, range.startColumn, range.startLineNumber, range.startColumn)); |
There was a problem hiding this comment.
I didn't understand this line, wouldn't this produce duplicate cursors?
|
@rebornix Sorry for the delay, I have been very busy, I had some spare time to look at this, and added 2 commits, let me know what you think! |
|
any updates on this? |
|
@alfonsoperez thanks for your contribution ❤️ |
Related issue: #17143,
Let me know what you guys think, I would love to have this feature.