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

Issue Tracker: Replace altercommand #1260

Open
1 of 3 tasks
theol0403 opened this issue Jul 4, 2023 Discussed in #887 · 11 comments
Open
1 of 3 tasks

Issue Tracker: Replace altercommand #1260

theol0403 opened this issue Jul 4, 2023 Discussed in #887 · 11 comments
Assignees
Labels
category: integration bindings, commands, etc enhancement New feature or request manager: commands priority

Comments

@theol0403
Copy link
Member

theol0403 commented Jul 4, 2023

The altercommand hack is very gross. There is some useful information in #887 that should help with removing it.

@theol0403 theol0403 added enhancement New feature or request priority labels Jul 4, 2023
@theol0403 theol0403 self-assigned this Jul 4, 2023
@theol0403 theol0403 changed the title Issue Topic: Replace Altercommand Issue Tracker: Replace Altercommand Jul 4, 2023
@theol0403 theol0403 changed the title Issue Tracker: Replace Altercommand Issue Tracker: Replace altercommand Jul 5, 2023
@theol0403
Copy link
Member Author

theol0403 commented Jul 5, 2023

@justinmk some early questions.

I understand how BufReadCmd can be used to intercept :edit and let vscode handle loading the file.

However, what about commands like :vsplit? In this case, vscode does not sync window position with neovim. That would be too complex. Thus, workbench.action.splitEditorRight needs to be called at some point.

Is there a WindowSplitCmd autocommand? Alternatively, is there a way for the BufReadCmd to know whether it was called by :vsplit? I suppose that would be ideal!

@theol0403
Copy link
Member Author

theol0403 commented Jul 5, 2023

Additionally, what about <C-w>v? The keybindings are less gross, but it would be good if it would trigger an autocommand we could use to call workbench.action.splitEditorRight.

@theol0403
Copy link
Member Author

theol0403 commented Jul 6, 2023

Same question with :saveas.
When BufWritePre is called, how can I know to call the vscode saveas command?

Edit: I guess in this situation I could compare the current filename vs <afile> and trigger a saveas.

@theol0403
Copy link
Member Author

@justinmk bump.

It looks like :vsplit does not trigger BufReadPre, so not sure how to intercept that.

@theol0403
Copy link
Member Author

theol0403 commented Jul 10, 2023

In neovim, I tried collecting as much information as possible. However, this is the data I get on a :split command with WinNew - I can't figure out how to know what kind of split it is.

maybe I can try to figure out the position of the new window and calculate the most likely split, but that seems like a lot of work. However, a window position sync api does seem like it could work well with some nvim plugins.

image

@justinmk
Copy link
Collaborator

justinmk commented Jul 16, 2023

Is there a WindowSplitCmd autocommand?

WinNew (as you noticed)

Alternatively, is there a way for the BufReadCmd to know whether it was called by :vsplit? ... the data I get on a :split command with WinNew - I can't figure out how to know what kind of split it is.

There is getwininfo(win_getid()) and win_screenpos() which can be used to intuit the layout. Vim itself doesn't know what command created the window, it only knows the dimensions.

I see the window layout stuff as a lower priority. Mappings are the right approach there, for now.

The main priority imo is to remove complicated layers of hacks except where absolutely necessary. :edit and :write and more useful than window layout.

  • My thought in #887 was that :edit can work properly without any hacks, just normal handling of BufReadCmd.
  • I assume #521 will "just work" without any special handling, after we remove the :Write hack.
  • #798 is tricky because of the "scratch" buffer.
    • Reviewing #887, I'm still wondering why a scratch buffer is needed. Can we instead open the actual filepaths in nvim, and maybe set buftype=acwrite + BufWriteCmd handler?

@theol0403
Copy link
Member Author

Thanks for the advice! Yeah I was hoping that this effort could entirely remove altercommand but I guess only partially for now.

Regarding letting nvim open the file, I think it's not possible given the reasons Alexey mentioned. That's because not all files are on the local filesystem (wsl, remote workspaces, liveshare, etc).

@justinmk
Copy link
Collaborator

justinmk commented Jul 17, 2023

Regarding letting nvim open the file, I think it's not possible given the reasons Alexey mentioned. That's because not all files are on the local filesystem (wsl, remote workspaces, liveshare, etc).

For those cases :!node % wouldn't work anyway. But that doesn't stop us from always having a valid name for the scratch buffer while using buftype=acwrite + BufWriteCmd to handle any nvim-side "write" commands. Remote files can be prefixed with a scheme like remote:// or whatever (or the random __vscode__:file:// thing), while any files that vscode.Uri.file() can resolve can have a normal local filepath?

IOW:

  • the "scratch buffer" approach can still be used,
  • but the buffer name doesn't need to be nonsense (for local files, at least), and
  • buftype=acwrite + BufWriteCmd should work just fine with the current "scratch buffer" approach.

@theol0403
Copy link
Member Author

Sounds good, hopefully I will have time to look into this before the summer runs out.

FWIW, this will also enable improvements with shada/undo history.

@theol0403
Copy link
Member Author

@xiyaowong this might be a good PR for v1.0.0 as well, if you are interested. Removing as many (probably not all) altercommands as possible will remove a lot of hacks.

@theol0403
Copy link
Member Author

Removing altercommands for vsplit and family will require neovim/neovim#13504

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: integration bindings, commands, etc enhancement New feature or request manager: commands priority
Projects
Status: Todo
Development

No branches or pull requests

2 participants