Issue Tracker: Replace altercommand

[theol0403]

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

• Allow write to send buffer to external program #521
• typing :vis makes neovim behave like it's in an empty file #752
• Current file name in % register is invalid #798

[theol0403]

@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]

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]

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]

@justinmk bump.

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

[theol0403]

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.

[justinmk]

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]

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]

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]

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]

@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]

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

[ollien]

Moving some discussion from here #2027 (review)

I was doing some investigating today, and this got a bit thornier. In response to @theol0403's roadblocks

how to associate a buffer with a URI?

I think we already do this here

vscode-neovim/src/buffer_manager.ts

Lines 613 to 614 in 350a35c

	vim.api.nvim_buf_set_var(bufId, "vscode_uri", docUri)
	vim.api.nvim_buf_set_var(bufId, "vscode_uri_data", docUriJson)

The only challenge is changing that URI when it changes (e.g. after a "save as"), but I think we could do that with workspace.onDidSaveTextDocument.

How to avoid saving Untitled files with :wall, when nvim triggers a save for every file individually?

Quite easily, actually. We can modify the above to be something like this:

diff --git a/src/buffer_manager.ts b/src/buffer_manager.ts
index 0ad931b..6023385 100644
--- a/src/buffer_manager.ts
+++ b/src/buffer_manager.ts
@@ -588,18 +588,11 @@ export class BufferManager implements Disposable {
      */
     private async initBufferForDocument(document: TextDocument, buffer: Buffer, editor?: TextEditor): Promise<void> {
         const bufId = buffer.id;
-        const { uri: docUri } = document;
-        logger.log(docUri, LogLevel.Debug, `Init buffer for ${bufId}, doc: ${docUri}`);
+        logger.log(document.uri, LogLevel.Debug, `Init buffer for ${bufId}, doc: ${document.uri}`);
 
         const eol = document.eol === EndOfLine.LF ? "\n" : "\r\n";
         const lines = document.getText().split(eol);
-        // We don't care about the name of the buffer if it's not a file
-        const bufname =
-            docUri.scheme === "file"
-                ? config.useWsl
-                    ? await actions.lua("wslpath", docUri.fsPath)
-                    : docUri.fsPath
-                : docUri.toString();
+        const bufname = await this.bufnameForTextDocument(document);
 
         await this.client.lua(
             `
@@ -612,18 +605,21 @@ export class BufferManager implements Disposable {
             vim.api.nvim_buf_set_var(bufId, "vscode_editor_options", vscode_editor_options)
             vim.api.nvim_buf_set_var(bufId, "vscode_uri", docUri)
             vim.api.nvim_buf_set_var(bufId, "vscode_uri_data", docUriJson)
-            vim.api.nvim_buf_set_name(bufId, bufname)
             vim.api.nvim_buf_set_option(bufId, "modifiable", not isExternalDoc)
-            -- force nofile, just in case if the buffer was created externally
-            vim.api.nvim_buf_set_option(bufId, "buftype", "nofile")
+            -- force acwrite, which is similar to nofile, but will only be written via the BufWriteCmd autocommand.
+            vim.api.nvim_buf_set_option(bufId, "buftype", "acwrite")
             vim.api.nvim_buf_set_option(bufId, "buflisted", true)
+
+            if bufname ~= vim.NIL then
+                vim.api.nvim_buf_set_name(bufId, bufname)
+            end
         `,
             [
                 bufId,
                 lines,
                 makeEditorOptionsVariable(editor?.options),
-                docUri.toString(),
-                docUri.toJSON(),
+                document.uri.toString(),
+                document.uri.toJSON(),
                 bufname,
                 this.isExternalTextDocument(document),
             ],
@@ -638,6 +634,18 @@ export class BufferManager implements Disposable {
         actions.fireNvimEvent("document_buffer_init", bufId);
     }
 
+    private async bufnameForTextDocument(doc: TextDocument): Promise<string | undefined> {
+        if (doc.isUntitled) {
+            return undefined;
+        } else if (doc.uri.scheme === "file" && config.useWsl) {
+            return await actions.lua("wslpath", doc.uri.fsPath);
+        } else if (doc.uri.scheme === "file") {
+            return doc.uri.fsPath;
+        } else {
+            // We don't care about the name of the buffer if it's not a file
+            return doc.uri.toString();
+        }
+    }
     /**
      * Create new neovim window
      */

By not giving the buffer a name, we allow nvim to handle unsaved buffers for us (e.g. it won't send a BufWriteCmd autocommand).

Do we need to remove the w! functionality for saveas, because there is no nvim equivalent?

Yes, probably.

[how to] saveas with a new URI?

I think this, above all else, is the hard problem, for a couple of reasons.

1. I'm not seeing any APIs VSCode exposes to us to give a name to an untitled file; we can only force a user to choose a new one in the "save as" dialog. We could perhaps do something hacky where we write to the filesystem directly and then open that new file but that will have undesirable effects (e.g. causing the user to lose their undo stack).
2. If a user has a file open (not a folder open), and they run :w myfile.txt, where should myfile.txt be located? Relative to the CWD of nvim may not always be predictable to a user (the default is whatever node's PWD is).

Maybe there's an easy answer to the above that I'm not seeing, but I do think we are limited by VSCode a bit.

[ollien]

what use case (i.e. specific :write invocation) are we trying to solve here?

If you open a new file (just hit ctrl+n), and then run :w /tmp/myfile.txt, I would want VSCode to write /tmp/myfile.txt to disk, just as if I had hit ctrl+shift+s, and given it the name myfile.txt in /tmp. This would change the open editor to /tmp/myfile.txt such that subsequent :ws would save to that same path.

If there is no vscode workspace/folder, vscode defaults to home dir (not sure if this is configurable).

Is this an already existing VSCode thing? Never seen it. Just saw your edit. I feel like that's a bit different, because then the user is actively acknowledging the directory for their file. I wouldn't expect :w myfile.txt to write to ~/myfile.txt unless I specified that.

I suppose one thing we could do is throw an error on write if given an absolute path with no open folder...? Not amazing but not awful.

open each buffer as its own tab, then we can set a tab-local CWD based on the vscode workspace for the given file.

This is an interesting approach, but I'm not sure that I understand what benefit the tab-local CWD gives us. If we already have a workspace, I think we have a pretty easy default of just joining the file name to the workspace root already, right?

[justinmk]

If you open a new file (just hit ctrl+n), and then run :w /tmp/myfile.txt, I would want VSCode to write /tmp/myfile.txt to disk, just as if I had hit ctrl+shift+s, and given it the name myfile.txt in /tmp.

Got it. There is a way to pre-fill the directory + filename in the "save as" dialog, so the worse case is simply that the user hits "OK". Seems acceptable to me.

I wouldn't expect :w myfile.txt to write to ~/myfile.txt unless I specified that.

If you open an untitled file in vscode without choosing a folder/workspace then you can't really have any other expectation 😄 (seriously though).

[ollien]

Got it. There is a way to pre-fill the directory + filename in the "save as" dialog, so the worse case is simply that the user hits "OK". Seems acceptable to me.

Do you know how to do that off hand? The only thing I see is workspace.saveAs(uri), but that requires the file already be saved as something, and doesn't seem to do prefill

If you open an untitled file in vscode without choosing a folder/workspace then you can't really have any other expectation 😄 (seriously though).

Haha, I'd rather an error than it go waves hands somewhere but I hear you. Minor point, though :) The untitled files is the big one

[theol0403]

What if you constructed a new uri to pass to saveAs? It's not some random ID but instead a structured definition of some resource.

[ollien]

What if you constructed a new uri to pass to saveAs? It's not some random ID but instead a structured definition of some resource.

I did try this with URI.file(path). I think you're falling into the same trap I did. workspace.saveAs(uri) does not say "I want to 'save as' to this new URI". It says "I want to execute 'save as' on this already opened resource".

[justinmk]

Do you know how to do that off hand?

SaveDialogOptions.defaultUri should set the directory. Hopefully setting the last segment to a non-existing file, will pre-fill the filename...

https://code.visualstudio.com/api/references/vscode-api#SaveDialogOptions

[theol0403]

Humm. Worth noting it doesn't currently work either so this wouldn't be a step backwards. Maybe we just call the dialogue when bang or a different path to the current URI is passed.

[ollien]

Do you know how to do that off hand?

SaveDialogOptions.defaultUri should set the directory. Hopefully setting the last segment to a non-existing file, will pre-fill the filename...

code.visualstudio.com/api/references/vscode-api#SaveDialogOptions

Nice spot @justinmk! This does exactly what you'd expect.

const filenameURI = Uri.file(filename);
window.showSaveDialog({
    defaultUri: filenameURI,
});

EDIT: jk, It almost does what you expect. It opens the dialog, but that's it. The save action does nothing. Lame.

[theol0403]

Actually, take a look at

https://marketplace.visualstudio.com/items?itemName=sleistner.vscode-fileutils
https://marketplace.visualstudio.com/items?itemName=patbenatar.advanced-new-file
https://marketplace.visualstudio.com/items?itemName=robole.file-bunny

[theol0403]

This might be what we need:

await vscode.workspace.fs.writeFile(uri, new Uint8Array());
await vscode.commands.executeCommand("vscode.open", uri);

[xiyaowong]

This might be what we need:

await vscode.workspace.fs.writeFile(uri, new Uint8Array());
await vscode.commands.executeCommand("vscode.open", uri);

Saving in this way won't trigger any actions executed by VS Code upon saving.

[ollien]

Yeah, it also probably won't work with anything other than the most basic cases (e.g. remote dev won't work). I'm gonna poke at this a bit and try to get a first-pass PR up soon

[theol0403]

https://code.visualstudio.com/api/references/vscode-api#FileSystem

vscode.workspace.fs can interact with remote filesystems. The best approach is likely only to use it when renaming/saveas, then using the regular save commands otherwise.