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

feat: passing multile of the same files in the arguments places a cursor at each position #12192

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

NikitaRevenco
Copy link
Contributor

@NikitaRevenco NikitaRevenco commented Dec 5, 2024

This PR allows files passed from the command line to be opened with multiple cursors. For example, previously the following will open README.md at line 8 with only 1 cursor:

hx README.md:4 README.md:6:3 README.md:8

Now, it'll open 3 cursors at README.md, each at line 4, line 6 column 3 and line 8 respectively.

why?

For example, let's say I want to open in Helix each time the word unsafe appears in Helix's codebase.

To get a list of all files, rows and columns with that match the query we can use ripgrep and awk:

rg --vimgrep unsafe | awk -F: '{print $1 ":" $2 ":" $3}'

The above command outputs all occurences of unsafe in this format, separated by newlines:

helix-view/src/document.rs:428:19
helix-lsp-types/src/lib.rs:18:11
helix-lsp-types/src/code_action.rs:210:9
runtime/queries/nix/highlights.scm:25:841
runtime/queries/nix/highlights.scm:25:873
runtime/queries/nix/highlights.scm:25:902
...

In total, there are 78 entries. If you use variable expansion to open each entry in Helix like this:

hx $(rg --vimgrep unsafe | awk -F: '{print $1 ":" $2 ":" $3}')

Previously, Helix would only place a single cursor at each file. With this PR, it'll place a cursor for each file argument with a position passed via the command line, at the exact location.

Now, you can use ctrl + S to add all cursors to the jumplist, and navigate between them to make your changes one-by one. Very handy!


this PR makes large scale context-based refactoring much easier because you can search for a pattern and place a cursor at each occurence of that pattern.

A lot of tools use the "vimgrep" format to output filename:line:column-number so if Helix can properly utilize those tools it would seriously makes Helix much more powerful

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

It shouldn't be possible to have a file open multiple times and Editor::open protects against this.

For this use-case instead we could add a cursor at each line(+col) passed as args

@NikitaRevenco
Copy link
Contributor Author

NikitaRevenco commented Dec 5, 2024

It shouldn't be possible to have a file open multiple times and Editor::open protects against this.

For this use-case instead we could add a cursor at each line(+col) passed as args

If I use a tool which reports spelling errors and it opens a file with e.g. 10 cursors each placed at a misspelled word, I need to be able to perform a small edit at each individual word, and manually judge how each should be changed

One can use ) and ( to change the currently focused cursor backward/forward, but for this to properly work I should also be able to collapse all cursors with , and then restore them back so i can jump to the next misspelled word

Do you see what I mean? is something like this currently possible? If there aren't any workarounds then it defeats a lot of utility of the feature because it would only be useful to mass-change those 10 words

@NikitaRevenco
Copy link
Contributor Author

NikitaRevenco commented Dec 5, 2024

basically it's important that even if we do only open 1 buffer per file, I should be able to easily jump between the line:column of each individual file. if i collapse all 10 cursors to 1 cursor to change a word, I should somehow be able to restore them so I can continue my refactor for the rest of the 9 misspelled words. wondering if something like this is currently possible

@Dryadxon
Copy link

Dryadxon commented Dec 5, 2024

basically it's important that even if we do only open 1 buffer per file, I should be able to easily jump between the line:column of each individual file. if i collapse all 10 cursors to 1 cursor to change a word, I should somehow be able to restore them so I can continue my refactor for the rest of the 9 misspelled words. wondering if something like this is currently possible

It should be doable by saving all the positions in the jumplist and moving between them as needed

@the-mikedavis
Copy link
Member

Yeah currently the jumplist is probably the best way to do this. If README.md:1 README.md:3 started with two cursors you could save these with C-s, edit one and then use C-o to restore the selection. Marks (#703) is similar and is probably the long-term solution.

Specifically my issue with the change as-is is opening multiples of the same file. That might break some internal assumptions (not sure about this part) and definitely breaks external assumptions. For example a language server assumes that a document is only being modified once. If we have multiples of a buffer open we'd have to send multiple textDocument/didOpens which the spec forbids, and diverging changes to the buffers would make the rest of document sychronization nonsensical anyways. Also consider the spell correction example - if you have multiple open buffers you need to :reload every time you make a correction and save since buffers are assumed to be independent and edits are not shared.

It does seem potentially useful to provide the same file multiple times in the argv with different locations - we just need to find a way to present that without opening duplicate buffers. Multiple cursors could be one way. Another idea could be opening a picker with the files in the argv rather than opening any files when we see a duplicate with a different location.

@NikitaRevenco
Copy link
Contributor Author

NikitaRevenco commented Dec 5, 2024

Yeah currently the jumplist is probably the best way to do this. If README.md:1 README.md:3 started with two cursors you could save these with C-s, edit one and then use C-o to restore the selection. Marks (#703) is similar and is probably the long-term solution.

Specifically my issue with the change as-is is opening multiples of the same file. That might break some internal assumptions (not sure about this part) and definitely breaks external assumptions. For example a language server assumes that a document is only being modified once. If we have multiples of a buffer open we'd have to send multiple textDocument/didOpens which the spec forbids, and diverging changes to the buffers would make the rest of document sychronization nonsensical anyways. Also consider the spell correction example - if you have multiple open buffers you need to :reload every time you make a correction and save since buffers are assumed to be independent and edits are not shared.

It does seem potentially useful to provide the same file multiple times in the argv with different locations - we just need to find a way to present that without opening duplicate buffers. Multiple cursors could be one way. Another idea could be opening a picker with the files in the argv rather than opening any files when we see a duplicate with a different location.

Ok, so now it will open a cursor at each file inputted!

I'm using an IndexMap with the keys being file names and the values being a vector of Positions, due to the fact that IndexMap preserves order of insertion

Let me know if there should be any more changes

@NikitaRevenco NikitaRevenco changed the title feat: allow multiple buffers to be open for the same file when passed via command line feat: passing multile of the same files in the arguments places a cursor at each position Dec 5, 2024
@NikitaRevenco
Copy link
Contributor Author

NikitaRevenco commented Dec 18, 2024

Ok, i just resolved the merge conflicts. this is good to be reviewed now

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.

3 participants