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

acwrite and protocol bufs (e.g. octo://, http://, scp://) are autosaved #8

Open
tmillr opened this issue Apr 21, 2023 · 0 comments
Open
Labels
breaking change ❕ urgent Very important or time-sensitive priority: high Important, needs current attention

Comments

@tmillr
Copy link
Owner

tmillr commented Apr 21, 2023

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
@tmillr tmillr added ❕ urgent Very important or time-sensitive priority: high Important, needs current attention breaking change labels May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ❕ urgent Very important or time-sensitive priority: high Important, needs current attention
Projects
None yet
Development

No branches or pull requests

1 participant