Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moderate improvements #17

Closed
wants to merge 24 commits into from
Closed

Moderate improvements #17

wants to merge 24 commits into from

Conversation

FreddieOliveira
Copy link

@FreddieOliveira FreddieOliveira commented Jul 15, 2017

Don't know why, but my last 7 commits are listed again, probably you merged them in a non conventional way, or maybe it was my mistake while updating my fork. Anyway, the new modifications are:

  • Now copy and cut operations will copy/cut the selected region OR the intire line if no region is selected (just as the original C-c and C-x behaviour)
  • Change kill to end of line implementation and add kill to beginning of line. Also, both kill operations will cut the line instead of just deleting it (jus like in emacs)
  • Add cursor go back operation with and without selection (C-x C-x and C-u C-SPC respectively)
  • Fix C-f to work as a cursor movement inside integrated terminal
  • Adjust some coding styles, like spaces/tabs, missing and trailing spaces, etc
  • Now deleteLine will not simply delete the current line, but in fact cut it (just like emacs original behaviour)
  • Rename deleteLine and kill functions to match emacs correspondent command name
  • Add C-x s shortcut to save all files

Some modifications in the code were necessary, like moving enterMarkMode and exitMarkMode to Editor class. Also, the markMode logic has changed: no more initSelection() call, just your vscode.commands.executeCommand( Editor.getInMarkMode() ? element+"Select" : element ); is enough. Other minor improvements in the code were performed as well.

src/editor.ts Outdated
return t
cut(): void {
vscode.commands.executeCommand("editor.action.clipboardCutAction")
Editor.inMarkMode = false
Copy link
Author

@FreddieOliveira FreddieOliveira Jul 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely if you permorf a "cancelSelection" after the "clipboardCutAction" the selected region will be cutted, but the whole line will disappear, probably it's a vscode bug. Tried to use async-await with no success.

src/editor.ts Outdated
exitMarkMode() : void {
vscode.commands.executeCommand("cancelSelection");
Editor.inMarkMode = false;
}
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes more sense and allows interesting things to be coded more easily to have these functions here instead of in extensions.ts file

@SebastianZaha
Copy link
Owner

Hmm, I am not sure that the copy paste action works now on all platforms completely. Initially a separate npm module was used because of cross-platform compatibilities and quirks.

I cannot test this on linux & windows right now as I am traveling but I'd like to take a closer look to see if it all works as expected.

I am not quite sure how to review and merge such a large amount of commits in one go, for the future it would be better if we made a PR for each individual change, for better clarity.

@FreddieOliveira
Copy link
Author

FreddieOliveira commented Jul 18, 2017

Yeah, I agree that's a lot of commits, sorry for the long PR list, next time I'll take it slow. I just tested things on Linux, can't say nothing for Windows right now, but I know what you mean:
microsoft/vscode#6994
Indeed in commit Change kill, copy and cut behaviour - 52c9a6c the clipboard module was removed and the builtins copy, paste and cut functions from vscode modules were used instead. Everything working fine in Linux, from what I can tell.

src/editor.ts Outdated
}

copy(): void {
vscode.commands.executeCommand("editor.action.clipboardCopyAction")
vscode.commands.executeCommand("emacs.exitMarkMode")
}

cut(appendClipboard?: boolean): Thenable<boolean> {
if (appendClipboard) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The append option was used because successive C-k cut operations from the same location append to the clipboard instead of just replacing it.

@@ -530,6 +530,11 @@
},
{
"key": "ctrl+x ctrl+s",
"command": "workbench.action.files.saveAll",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in emacs this is the other way around, right? Ctrl+x Ctrl+s is save buffer, and ctrl+x s is save all.

@SebastianZaha
Copy link
Owner

I tried to cherry pick some things from here but it is quite difficult to see the changes since some commits refer to other previous commits in the pull request.

Could I ask to rebase on top of the current master and re-make the pull requests one-per-feature so we can review them?

@SebastianZaha
Copy link
Owner

Closing after long inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants