vim icon indicating copy to clipboard operation
vim copied to clipboard

Adding register viminfo partial write warning

Open ElectricPulse opened this issue 1 year ago • 8 comments

The following often happens to me: I cut ("c" command) 100+ lines from file1, close file1, paste into file2, realize I had lost 50+ lines of text, which can't be recovered from file1 because it was cut out and the file closed. I realize that this problem has many solutions (use one vim session, :set viminfo, use system clipboard &c) yet this still happens to me, especially on machines where I forgot to :set viminfo in vimrc and where there is no system clipboard. For the sake of noobs I decided to atleast give a warning when such a thing is about to happen.

This feature request tries to address this in the following ways:

  1. Add a warning when exiting with register size bigger than viminfo
  2. Increasing viminfo register max line count from default 50 lines to 500 lines

Note 1: Regarding the warning, it doesn't prevent the user from exiting, it just displays the messages and after pressing Enter, exits. This is because even though I changed it so that do_viminfo now returns a FAIL or OK status, write_viminfo doesn't return such an error because I have no idea of how about implementing the more intrusive feature of an getout failing, ie. ex_exit failing.

Note 2: What error number should be applied to the new warning (I set it to 2000)?

ElectricPulse avatar May 16 '24 14:05 ElectricPulse

It might be a good idea to add a description of this warning. For example, here: https://vimhelp.org/options.txt.html#%27viminfo%27

RestorerZ avatar May 19 '24 10:05 RestorerZ

Fixed the tests.

ElectricPulse avatar May 21 '24 13:05 ElectricPulse

Note 1: Regarding the warning, it doesn't prevent the user from exiting, it just displays the messages and after pressing Enter, exits. This is because even though I changed it so that do_viminfo now returns a FAIL or OK status, write_viminfo doesn't return such an error because I have no idea of how about implementing the more intrusive feature of an getout failing, ie. ex_exit failing.

I checked this and this got me wondering. I see, that there is already some error handling, that simply shows an error message but does still exit Vim after the user presses Enter. So I am wondering how useful this is? It's certainly too late for the user to do anything against this and fix the warning. But on the other side, with the newly increased defaults, perhaps this is something not to worry about it for now?

chrisbra avatar May 21 '24 21:05 chrisbra

I did check the vim-history repo and the s flag (max size of registers) has been added to the viminfo option as of patch 6.2.393 (from around Mar 2004). So I think, increasing the limit by 10 times to 100 KBytes should be okay after more than 20 years. I didn't try to find when the </" item has been set initially. So all in all I think this is good change

chrisbra avatar May 23 '24 19:05 chrisbra

@ElectricPulse are you going to address those few points please?

chrisbra avatar Jun 02 '24 18:06 chrisbra

@ElectricPulse are you going to address those few points please? Yes, it is a macos test failing and my macos vm is taking forever to install.

ElectricPulse avatar Jun 03 '24 19:06 ElectricPulse

This one:

Expected {'id': 15, 'jsonrpc': '2.0', 'result': 'delayed-payload'} but got {}

is a known flaky one.

Not sure about Test_popupwin_beval_3.dump for macos 12, huge. Let me try to re-trigger this one.

chrisbra avatar Jun 03 '24 19:06 chrisbra

A late suggestion, but one I hope will be considered: perhaps this is better hooked into 'confirm'? I know current documentation says it's about fallible operations due to unsaved changes, but this feels like it's in that vein of problems (give me the opportunity to notice that something is wrong when quitting).

In fact, I suspect most of the warning + prompt code is already available in the confirm implementation; this would only need to perform the check from this PR in the right spot?

benknoble avatar Jun 16 '24 20:06 benknoble

closing for now. If you ever finish this up, please open a new PR. thanks

chrisbra avatar Nov 12 '24 20:11 chrisbra