tmillr / sos.nvim Goto Github PK

What is the issue?

acwrite and protocol bufs (e.g. those with a name of octo://, http://, scp://) are autosaved by both the plugin and vim's autowrite options.

Why/how is this an issue?

Fugitive's fugitive://* can stage files when written (i.e. alter the index); this may go undetected by the user (especially when the write is done automatically), and at best is a nuisance and probably hard to recover once the buffer is gone and undo history is lost. Fugitive will also write to stage 1-3 (aka unmerged) index entries if the buf being written represents such an index entry. Most of these bufs appear to have buftype set to empty string '' (and not acwrite).

Octo buffers octo://* can post (potentially unfinished) comments/content to GitHub (via web request) when written.

netrw bufs can also send web requests, which might be slow, or use up a limited data allotment, etc. Bufs may also be opened up on a directory, and different plugins may use different strategies, buf names, and vim options for such buffers. netrw appears to use a normal/empty buftype on dir listings, without a name/path, and with readonly set.

Many users probably want to allow only manually saving of such buffers.

Ideas/Possible Solutions

1. Ignore in sos (don't write/save them), and instruct users to disable all autowrite options.

   • If they are (only) ignored in sos, then autowrite and autowriteall vim options will still autosave them. In this case the user would need to disable those options (which atm combine well with this plugin), and then sos would need to replicate the behavior of those opts but with the exception of ignoring special/acwrite/protocol bufs (behavior which could either be enabled by default, or via config).

2. Simply mark such buffers as readonly (buffer-local vim option). They then should automatically get ignored by both the plugin and vim's autowrite* opts, and manual writing can be achieved with :confirm write, :write!, or a :write whilst confirm is set. By default, nvim unsets readonly on write, but this can be changed via cpo. A caveat of this is the fact that :write! not only silently overrides readonly, but also anything else that would have otherwise prevented the write from being attempted (e.g. the file changed both in vim and on fs).

3. Same as 2, but instead of doing it in the plugin, instruct users to do it via their own autocmd. The user can then opt-in to this behavior on their own, with the default being to autosave such buffers.

If the writing of such buffers is ignored globally (e.g. via BufWrite* autocmds), it won't be possible to write them manually (unless the autocmds contain conditional code that e.g. checks for bang !, or can otherwise distinguish between a manual and automatic :write, etc.), but it would probably also handle writes done by autowrite. Most of these special buffers already define a BufWriteCmd autocmd however, which (according to vim's docs) automatically disables any BufWritePre and BufWritePost autocmds (and it is then up to the BufWriteCmd autocmd itself to trigger them manually at the appropriate times). Caveat: preventing the write by throwing inside of a BufWritePre autocmd will display an error message every time. Also, the BufWriteCmd autocmd may forget to manually trigger the corresponding Pre and Post autocmd events altogether.

Conclusion: both 1 and 2 can be implemented. 2 can be made opt-in or opt-out via config setting. 2 might be a slight annoyance for the user, as writing a readonly buf requires bang or confirm, the latter of which will prompt every time (if Z is in cpo). However, when writing a buffer may do things such as post comments online or alter the git index, such a warning prior to writing may come to be a worthy and welcomed addition.

Detection

filetype and/or syntax can also be used to identify these special buffers as they usually have a particular filetype and highlighting, but probably not reliably.

buftype settings are inconsistent (e.g. fugitive), although if set to acwrite it's probably safe to assume that writing will be done via autocmd. It appears possible to have a BufWriteCmd autocmd defined with a non acwrite buftype, and nvim does not set buftype automatically.

These type of buffers are usually not readonly as they are usually allowed to be saved/written (it's just that the writing is done differently, or not at all in which case a completely different action will be taken). It may be possible to check for registered BufWrite autocmds before writing, although this might not apply to all kinds of special bufs (e.g. netrw's dir listings, which do have readonly set though), and might be slow (although the result could be cached/memoized if necessary).

The bufname can be checked (e.g. check for a protocol name).

Conclusion: use a combination of checking: buftype, BufWriteCmd, and bufname (but not buftype alone)

Disallow 'autowrite'?

Pros

• no need to force (ignored) bufs which shouldn't be autosaved (e.g. acwrite bufs) to be 'readonly', or otherwise risk autosaving a buffer which shouldn't have been

Cons

• sos won't be able to save prior to commands invoked via <Cmd> inside mappings, or those invoked via vim.cmd(), etc. as it relies on autocmds like CmdlineLeave for save_on_cmd, although a workaround could be to change <Cmd> to : or inject a custom command as needed. Although, any of the commands covered by autowrite which trigger BufLeave should already be detected by sos regardless of whether CmdlineLeave is triggered...needs testing.
• there may be future additions/enhancements to 'autowrite' that may not be able possible or feasible to cover/implement in sos

Todo

• benchmark performance before and after
• allow customization/configuration
• figure out what to do with/how to handle 'autowrite' and add tests to ensure that its functionality is covered/provided by sos

don't rely on/require the user to use 'autowrite' for this

• decide whether to make configurable

There are a couple of (potential) issues when using Plenary alone:

1. Tests are not isolated

• Every test in a spec file runs within the same nvim process/instance requiring manual sanitization between tests

2. Plenary runs tests in headless mode

• This is probably not an issue 90%+ the time, but it seems that Neovim's behavior does differ slightly when headless (particularly when it comes to the issuing of messages/prompts/etc.)

Suggestion: spawn an nvim process/job in each test and run the test in that. Either run the entire it() function within that process, or simply communicate with it over rpc (i.e. call fns and get/set values over rpc as needed, but assertions are still executed in the initial Plenary process).

• Fix anything broken by VimResume, mainly :checktime on resume
• Fix failing tests

1. Describe (in detail) in the README why someone might want to use this plugin, what it does/fixes, etc..
2. (Optional) Compare/contrast with vim's 'autowrite'

1. tests for TermLeave
2. ensure that it also fires when leaving buf directly (w/o exiting insert/terminal mode first)
3. ensure it also fires when switching to same buf (but to a different tab or window)

e.g. Luacov

Eg device crashes or runs out of battery while unnamed buffers were open in vim that you wanted preserved?

Potential Fixes

• Use/enable vim swapfiles, but only for unnamed buffers (as swapfiles in general are annoying)
• Custom swapfile implementation
• Call :mksession automatically (eg on timer and/or only under certain conditions)
• Emit reminder/notification to user telling them to to name/save unnamed buffers or convert them to scratch buffers

https://github.com/marketplace/actions/release-please-action