-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Adding register viminfo partial write warning #14787
base: master
Are you sure you want to change the base?
Adding register viminfo partial write warning #14787
Conversation
src/viminfo.c
Outdated
@@ -1906,8 +1910,10 @@ write_viminfo_registers(FILE *fp) | |||
fprintf(fp, "\t%s\t%d\n", type, (int)y_ptr->y_width); | |||
|
|||
// If max_num_lines < 0, then we save ALL the lines in the register | |||
if (max_num_lines > 0 && num_lines > max_num_lines) | |||
if (max_num_lines > 0 && num_lines > max_num_lines) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (max_num_lines > 0 && num_lines > max_num_lines) { | |
if (max_num_lines > 0 && num_lines > max_num_lines) | |
{ |
src/viminfo.c
Outdated
write_viminfo_registers(fp_out); | ||
|
||
if(write_viminfo_registers(fp_out) == FAIL) { | ||
emsg(_(e_warning_registers_partially_written_to_viminfo)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add tests for these changes to the testdir/test_viminfo.vim file?
src/errors.h
Outdated
EXTERN char e_warning_registers_partially_written_to_viminfo[] | ||
INIT(= N_("E2000: Warning: registers only partially written to viminfo, increase viminfo size")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood your description correctly, then this is not an error, but a warning.
Like this:
Line 2278 in 68f8cfb
emsg(_("W14: Warning: List of file names overflow")); |
Accordingly, the number should not start with the letter "E", but with the letter "W". The last warning number was "W22".
Note 2: What error number should be applied to the new warning (I set it to 2000)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh okay and why are there Warnings under src/errors.h?
It might be a good idea to add a description of this warning. |
Co-authored-by: K.Takata <kentkt@csc.jp>
Co-authored-by: K.Takata <kentkt@csc.jp>
Fixed the tests. |
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? |
@@ -2769,7 +2769,7 @@ static struct vimoption options[] = | |||
{(char_u *)"", | |||
(char_u *)"'100,<50,s10,h,rdf0:,rdf1:,rdf2:"} | |||
#else | |||
{(char_u *)"", (char_u *)"'100,<50,s10,h"} | |||
{(char_u *)"", (char_u *)"'100,<500,s100,h"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we change the default, I think we should also increase it for at least Windows (see above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options.txt should be also updated accordingly:
Lines 9032 to 9034 in 701ad50
MS-Windows: '100,<50,s10,h,rA:,rB:, | |
for Amiga: '100,<50,s10,h,rdf0:,rdf1:,rdf2: | |
for others: '100,<50,s10,h) |
redir => res | ||
silent! wviminfo Xviminfo | ||
redir END | ||
call assert_match('W23:', res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of :redir, you can use execute()
:
call assert_match('W23:', res) | |
let res = execute('wviminfo Xviminfo') | |
call assert_match('W23:', res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commited it along with the silent!
since that allows the capturing of the warning
Co-authored-by: Christian Brabandt <cb@256bit.org>
@@ -2895,17 +2905,18 @@ read_viminfo_up_to_marks( | |||
/* | |||
* do_viminfo() -- Should only be called from read_viminfo() & write_viminfo(). | |||
*/ | |||
static void | |||
static int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for returning a return value here? It seems like all callers of do_viminfo()
do not check the return value, so I guess it is no needed?
I did check the vim-history repo and the |
@ElectricPulse are you going to address those few points please? |
|
This one:
is a known flaky one. Not sure about |
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:
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)?