-
Notifications
You must be signed in to change notification settings - Fork 802
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
expand line command actions #1449
base: main
Are you sure you want to change the base?
Conversation
I dig it. I'd be interested in your feedback on this one too: #1424 (similar to vim's |
Ya, I'm a bit iffy on using find here, just happens to be what I had used in vim. I'd be fine with |
dce6231
to
abb2ee1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
I wanted to share an idea that I just had about this: it would be really cool if we could add a similar command for adding a cursor at the next glyph (meaning: leave a cursor where you are now and then add another one at the next instance of this glyph) |
I wish I could merge this for you 😄 |
can file an issue or open a separate PR for it. you'd want that to only add a cursor to the next glyph on the same line or next in the file? makes me wonder if there should be a file for 'multiple cursor' related commands. I don't ever use them aside from with cursorless, but I suspect lots of others do. would need some common command to indicate you want to add a cursor too. "curse" would be good but curse on/off is used for mouse cursor, so a bit confusing. but something like "curse add next char this"? You could just do what you want in cursorless already with something like (untested) "pre char this and next view char this", but it's a bit wordy. anyway probably better to discuss in a different issue or PR if you prefer not to use cursorless for that. |
👍fwiw you can also use cursorless to navigate to the next glyph. I use multi cursor most often when I don't have access to cursorless because I'm having to work in IntelliJ. |
We discussed this in the latest community session.
|
abb2ee1
to
93c9297
Compare
Changed the names and doc strings to reflect the feedback.
vimscript.py is for the vimscript language, not for the vim app, so not applicable there. community doesn't have any native support for vim atm. Adding it is non-trivial because of mode switching, so it's not as simple as just enabling it if vim is active somewhere. Most people using vim (afaik) used to use my personal repo, which then got broken out into talon-vim, and then partially got broken into a much simpler implementation neovim-talon, which is what will be paired with cursorless in neovim. The neovim-talon repo will use these actions being added in this PR, so I don't think there needs to be something in community using them right away. I suspect there is probably some emacs equivalent that could be added if you think there really should be something. Or we could add something for https://marketplace.visualstudio.com/items?itemName=ArturoDent.jump-and-select, but I doubt many people are using that in vscode (could be wrong). |
I did notice that zsh by default seems to bind a readline shortcut for doing this, but bash doesn't. So will need shell-specific readline overrides, which we don't support atm. I plan to add this to my branch and then port it over eventually after a bunch of testing, but not sure if it needs to be part of this PR. I don't mind to wait though if it's preferred to have some real use cases in community. I guess maybe of note, I have a vscode plugin scope in my branch so can enable certain commands if a vscode plugin is installed, which could be used to enable jump-and-select extension mentioned above to more cleanly add support for this into vscode as well. I haven't opened a PR to add that to community yet though. |
93c9297
to
0dc2b80
Compare
Update from the community session. We think that we should probably wait with this until we actually have an implementation in community that utilizes the spoken forms. |
No worries. I'll add something that uses it as part of this PR then. |
Ok the scope of this is expanding now... but this implements a plugins tag and associated scope so that you can now specify if an app has a specific plugin installed. I also include action implementation for the new line command if the jump-and-select plugin is installed in vscode. I haven't bothered moving all of the other plugin usage from vscode.talon to use this new Also atm the test if the plugin is installed is tested on Linux only. Will test on Mac later, but if someone on Windows can hopefully chime in about where the settings file is located I'll add that too. |
Notes from community session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works very well, and I think we should bring it in. Few minor details to sort through.
There were some concerns raised vis-a-vis parsing the extension file (performance?), but this approach makes sense to me.
if not extensions.exists(): | ||
return [] | ||
with extensions.open() as f: | ||
return [x["identifier"]["id"] for x in json.loads(f.read())] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return [x["identifier"]["id"] for x in json.loads(f.read())] | |
return [x["identifier"]["id"].lower() for x in json.loads(f.read())] |
Just in case there are case differences, perhaps we should standardize on lower case?
@@ -74,3 +74,6 @@ select camel left: user.extend_camel_left() | |||
select camel right: user.extend_camel_right() | |||
go camel left: user.camel_left() | |||
go camel right: user.camel_right() | |||
|
|||
go char <user.unmodified_key>: user.jump_cursor_to_next_char(unmodified_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure these should live in line commands. Maybe they make sense alongside the plugin and be enabled with the appropriate tag? As it is, this would add some commands that won't work in a lot of applications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrm. I do think there is the potential for this to be used elsewhere in community, though not sure of any good example where to add it off the top of my head. But for instance, I already used it in my repo for zsh line editor (would like eventually to PR that), and was going to adapt vim to use it after (though that won't become part of community) and similarly I guess people could add it for emacs (maybe there is no emacs in community). I added a separate vscode example in this PR to community specifically to justify it since we didn't want an otherwise completely dead command, but I do see your point..
Having it on the plugin side only doesn't entirely make sense due to the potential to be used inside community, but if you guys decide you don't want it in here and just want to lift the plugin stuff from this PR instead I'm good with that. Can always add it later once there is a second use case.
|
||
|
||
ui.register("win_focus", scope.update) | ||
ui.register("win_title", scope.update) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need win_title to update the scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrm, I think probably not. I don't entirely remember why I added that. This code was simplified from some vim plugin parsing I had written awhile back though, so it may have been required for vim, but I can't see why it would be for vscode. Will remove.
|
||
def vscode_update_plugins_list(): | ||
"""Queries the list of vscode plugins""" | ||
extensions = pathlib.Path.home() / ".vscode/extensions/extensions.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will not work for all the different clones of vscode. If you look at our application definitions we support applications like VSCodium and Cursor. Anything we add in the vscode file should ideally work on all of these.
|
||
@mod.scope | ||
def scope(): | ||
return {"plugin_installed": actions.user.plugins_list()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're thinking actions.user.plugins_list
should have a VScode exclusive implementation for now. For other variants, these could be added by folks.
Long term, we're thinking to explore getting this via the command server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've looked into this and there isn't a simple way from the command server to know this folder path, but we can return a list of all the extensions from the command server.
def jump_cursor_to_prev_char(char: str): | ||
"""Jumps the first to the previous instance of the specified character on the current line""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qq: could you elaborate why limiting search results to the current line is valuable? I thought it was because it would wrap so you could get to stuff closer to the beginning quickly, but it looks like in vim f
doesn't do that. Probably something we were missing. cc @nriley who tried it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I often fuzzily (as in I don't know the exact index so overshoot) jump to the last character on a line (and don't want to jump off the line) and also don't always want to have to speak multiple commands to go to the end of the line and then search backwards. Also helps if you are working on a single line and definitely don't want to go off that line in the event of a misrecognition/mispeak (a regularity for me).
If others don't think that's actually useful to general users happy to just close.
Alright, I wanna try to get this one moving again. My take My proposal:
|
This adds two new actions for lines that allow for something like
f<key>
in vim (and other places) which will just jump to the first occurrence of that character.The only debatable bit is likely the use of
find
as the command. Open to suggestions though.