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

Add support for copying lines up and down #23

Merged
merged 5 commits into from Jan 31, 2022
Merged

Add support for copying lines up and down #23

merged 5 commits into from Jan 31, 2022

Conversation

mk12
Copy link
Contributor

@mk12 mk12 commented Jan 30, 2022

I customized this to meet my needs better:

  • Changed Mod+J to Ctrl+J because VS Code uses Ctrl+J even on macOS.
  • Added Alt+Up/Down for moving lines like in VS Code.
  • Replaced Ctrl+Shift+D with Alt+Shift+Up/Down like in VS Code.

I realized you probably won't want to merge this as-is since it will break what users currently expect (Ctrl-Shift-D) but feel free to incorporate it somehow if you wish.

Also, I can't get tests to work. I'm getting TypeError: editor.transaction is not a function. But it works when I test the plugin manually.

src/actions.ts Outdated
Comment on lines 88 to 97
editor.transaction({
changes: [
{text: '', from: previousLineStart, to: fromLineStart},
{text: '\n' + contentsOfPreviousLine.trimEnd(), from: toLineEnd},
],
selection: {
from: { line: from.line - 1, ch: from.ch },
to: { line: to.line - 1, ch: to.ch }
},
});
Copy link
Owner

Choose a reason for hiding this comment

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

Does this allow you to group changes to be applied all at once instead of doing replaceRange and setSelection separately?

I've been meaning to look into editor.transaction as well but the Obsidian API for it seems to differ from the CodeMirror 6 API...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly. Before I had two separate replaceRange calls but then Cmd+Z was stepping through them individually. I tried to merge into one replaceRange call but that left the selection in the wrong state. Then I found transaction.

Copy link
Owner

Choose a reason for hiding this comment

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

Nice! transaction seems like it would be especially useful for multiple selections, which I'm working on right now.

@timhor
Copy link
Owner

timhor commented Jan 30, 2022

I'll keep the existing shortcuts to avoid introducing breaking changes since users can customise them in settings anyway. Mod+J and Mod+Shift+D currently match Sublime Text shortcuts instead of VS Code.

Tests would need a larger rework to pass since they're using CodeMirror 5 at the moment, which doesn't utilise transactions for state updates (which is what your implementation of move line up/down relies on).

Is the behaviour of moving lines different to the built-in functionality though (#13)? If not, perhaps you could use that instead of writing a custom version?

@mk12
Copy link
Contributor Author

mk12 commented Jan 30, 2022

I see, perhaps you could have a switch to let users choose "VS Code style" or "Sublime style" hotkeys, since most people looking for a specific one and finding this plugin.

No, "Swap line" is exactly what I wanted 🤦‍♂️ Not sure how I missed that. In that case all I need is copy up/down. I can try rewriting it without transaction.

@mk12
Copy link
Contributor Author

mk12 commented Jan 30, 2022

Ok, I've amended the patch to leave Mod+J alone and remove the "Move line" actions. I've left "Duplicate line" as a separate action but internally calling copyLineDown because they do the same thing.

I suppose you could also name them "Duplicate line up" and "Duplicate line down", leaving the former unset and using Mod+Shift+D for the latter. I think a big benefit of this plugin would be the convenience to get the same shortcuts you expect from a particular editor without fiddling around with key mappings, though.

@timhor
Copy link
Owner

timhor commented Jan 30, 2022

perhaps you could have a switch to let users choose "VS Code style" or "Sublime style" hotkeys

Yeah that's a good idea, I'll look into adding some plugin-specific settings in a future release.

src/main.ts Outdated Show resolved Hide resolved
@timhor
Copy link
Owner

timhor commented Jan 30, 2022

When performing copy line up at the end of a line, the cursor position remains unchanged.

copy-up-bug

@mk12
Copy link
Contributor Author

mk12 commented Jan 30, 2022

When performing copy line up at the end of a line, the cursor position remains unchanged.

Huh that's weird, I can't reproduce it. It works as expected for me.

@timhor
Copy link
Owner

timhor commented Jan 30, 2022

Hmm I can reproduce it consistently. I'm on Obsidian v0.13.23 with no other plugins except for this one.

It only happens when the cursor (or selection end) is after the last character of a line. For example, at the end of bbb, or from the end of bbb to the end of ccc.

@timhor timhor changed the title Add support for moving/copying lines up and down Add support for copying lines up and down Jan 31, 2022
@timhor timhor merged commit 9f624e5 into timhor:master Jan 31, 2022
@mk12 mk12 deleted the patch branch February 1, 2022 02:26
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.

None yet

2 participants