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 command to move files with LSP support #8584

Merged
merged 20 commits into from
Nov 8, 2023

Conversation

yo-main
Copy link
Contributor

@yo-main yo-main commented Oct 21, 2023

close #4393

Allows to rename the active buffer by moving it to another path.

:move my_new_file.rs

Heavily based on all the work done in #5531 by @ontley and apply the suggestions

The PR seems dead and I would really like to see that feature come into helix :)

@yo-main yo-main force-pushed the master branch 6 times, most recently from 7ffa81e to 88b3653 Compare October 22, 2023 09:03
@archseer
Copy link
Member

Can you keep the commit history and retain the original author then? If the PR is simply rebased or patched up I'd like to retain the original author so we properly attribute the work they've put in.

@ontley
Copy link
Contributor

ontley commented Oct 22, 2023

Can you keep the commit history and retain the original author then? If the PR is simply rebased or patched up I'd like to retain the original author so we properly attribute the work they've put in.

It's a big change compared to the code I had, this adds multiple lsp server support and it handles it properly unlike mine

helix-lsp/src/client.rs Outdated Show resolved Hide resolved
helix-term/src/commands/typed.rs Outdated Show resolved Hide resolved
helix-term/src/commands/typed.rs Outdated Show resolved Hide resolved
helix-term/src/commands/typed.rs Outdated Show resolved Hide resolved
helix-term/src/commands/typed.rs Outdated Show resolved Hide resolved
@yo-main
Copy link
Contributor Author

yo-main commented Oct 22, 2023

Can you keep the commit history and retain the original author then? If the PR is simply rebased or patched up I'd like to retain the original author so we properly attribute the work they've put in.

It's a big change compared to the code I had, this adds multiple lsp server support and it handles it properly unlike mine

Couldn't have manage the lsp thing without seeing how it was implemented in your PR :)
I managed to rebase my branch using your fork (that wasn't easy indeed 😄 I hope I didn't mess anything up)

@yo-main yo-main force-pushed the master branch 2 times, most recently from 624977c to 5807113 Compare October 22, 2023 15:17
@yo-main yo-main requested a review from pascalkuthe October 22, 2023 17:08
@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 23, 2023
@yo-main yo-main requested a review from ontley October 24, 2023 19:34
@pascalkuthe
Copy link
Member

this will probably need to wait for the next release, the current one is pretty close and I doubt we will merge new features before it

@pascalkuthe pascalkuthe added this to the next milestone Oct 24, 2023
alirezaayande92

This comment was marked as spam.

@the-mikedavis the-mikedavis merged commit e868678 into helix-editor:master Nov 8, 2023
6 checks passed
@cbr9
Copy link
Contributor

cbr9 commented Nov 9, 2023

Shouldn't this command allow to move files into directories by automatically adding the filename? Currently you have to manually type the filename if you want to move the buffer into a directory

@David-Else
Copy link
Contributor

I just tried this with ltex-ls and immediate problems :(

hx hello.md

(create some text and save)

:move bye.md

if you use move too fast (before the language server has finished working I assume) you get an error like:

2023-11-10T16:12:02.351 helix_lsp::transport [ERROR] ltex-ls err <- "WARNING: Could not find document with URI 'file:///home/david/Downloads/bye.md'\n"
2023-11-10T16:12:02.353 helix_lsp::transport [ERROR] marksman err <- "[16:12:02 WRN] <MarksmanServer> Document not found: {\"uri\": \"{ uri = \\\"file:///home/david/Downloads/bye.md\\\"\\n  data = AbsPath \\\"/home/david/Downloads/bye.md\\\" }\", \"method\": \"textDocumentDidChange\"}\n"
2023-11-10T16:12:02.460 helix_lsp::transport [ERROR] marksman err <- "[16:12:02 WRN] <MarksmanServer> Document not found: {\"uri\": \"{ uri = \\\"file:///home/david/Downloads/bye.md\\\"\\n  data = AbsPath \\\"/home/david/Downloads/bye.md\\\" }\", \"method\": \"textDocumentDidChange\"}\n"
2023-11-10T16:12:02.460 helix_lsp::transport [ERROR] ltex-ls err <- "Nov 10, 2023 4:12:02 PM org.bsplines.ltexls.server.LtexTextDocumentService getDocument\n"
2023-11-10T16:12:02.460 helix_lsp::transport [ERROR] ltex-ls err <- "WARNING: Could not find document with URI 'file:///home/david/Downloads/bye.md'\n"

If you wait for ltex-ls to fully load before moving then the new file then you get 'no code actions available' but the text seems to have inherited the old underline errors?

Anyway, something funny is going on with the language server in this case. I am running ltex-ls and marksman. Cheers.

@pascalkuthe
Copy link
Member

That is a language server problem. What we do on our side is quite trivial

danillos pushed a commit to danillos/helix that referenced this pull request Nov 21, 2023
* Added rename command

* Added an error if the new path already exists

* Fixed wrong command name being used

* fixed clippy suggestions

* removed didRenameFiles call, fixed early return due to path Err

* added ':rnm' alias to ':rename'

* code cleanup

* formatting

* removed debug line

* cargo fmt

* Improved new buffer error message

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Removed unnecessary path normalizing

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* feat: change `rename` command to `move`

* feat: add multi lsp support when moving files

* feat: allow lsp calls with a custom timeout

* feat: sending lsp file_changed event once file has moved

---------

Co-authored-by: ontley <theontley@gmail.com>
Co-authored-by: ontley <67148677+ontley@users.noreply.github.com>
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
* Added rename command

* Added an error if the new path already exists

* Fixed wrong command name being used

* fixed clippy suggestions

* removed didRenameFiles call, fixed early return due to path Err

* added ':rnm' alias to ':rename'

* code cleanup

* formatting

* removed debug line

* cargo fmt

* Improved new buffer error message

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Removed unnecessary path normalizing

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* feat: change `rename` command to `move`

* feat: add multi lsp support when moving files

* feat: allow lsp calls with a custom timeout

* feat: sending lsp file_changed event once file has moved

---------

Co-authored-by: ontley <theontley@gmail.com>
Co-authored-by: ontley <67148677+ontley@users.noreply.github.com>
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* Added rename command

* Added an error if the new path already exists

* Fixed wrong command name being used

* fixed clippy suggestions

* removed didRenameFiles call, fixed early return due to path Err

* added ':rnm' alias to ':rename'

* code cleanup

* formatting

* removed debug line

* cargo fmt

* Improved new buffer error message

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Removed unnecessary path normalizing

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* feat: change `rename` command to `move`

* feat: add multi lsp support when moving files

* feat: allow lsp calls with a custom timeout

* feat: sending lsp file_changed event once file has moved

---------

Co-authored-by: ontley <theontley@gmail.com>
Co-authored-by: ontley <67148677+ontley@users.noreply.github.com>
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
* Added rename command

* Added an error if the new path already exists

* Fixed wrong command name being used

* fixed clippy suggestions

* removed didRenameFiles call, fixed early return due to path Err

* added ':rnm' alias to ':rename'

* code cleanup

* formatting

* removed debug line

* cargo fmt

* Improved new buffer error message

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Removed unnecessary path normalizing

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* feat: change `rename` command to `move`

* feat: add multi lsp support when moving files

* feat: allow lsp calls with a custom timeout

* feat: sending lsp file_changed event once file has moved

---------

Co-authored-by: ontley <theontley@gmail.com>
Co-authored-by: ontley <67148677+ontley@users.noreply.github.com>
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* Added rename command

* Added an error if the new path already exists

* Fixed wrong command name being used

* fixed clippy suggestions

* removed didRenameFiles call, fixed early return due to path Err

* added ':rnm' alias to ':rename'

* code cleanup

* formatting

* removed debug line

* cargo fmt

* Improved new buffer error message

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Removed unnecessary path normalizing

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* Update helix-term/src/commands/typed.rs

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

* feat: change `rename` command to `move`

* feat: add multi lsp support when moving files

* feat: allow lsp calls with a custom timeout

* feat: sending lsp file_changed event once file has moved

---------

Co-authored-by: ontley <theontley@gmail.com>
Co-authored-by: ontley <67148677+ontley@users.noreply.github.com>
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename buffers
9 participants