fix: Use vim_dialog_yesno for the "file changed since reading" message
#29310
Conversation
|
This is my first PR in neovim core so I'm not sure if I should assign anyone or who I could reach out to for feedback, but feedback is very much appreciated! |
murtaza64
force-pushed
the
bufwrite-use-do-dialog
branch
5 times, most recently
from
de33f3f
to
e989b35
Compare
murtaza64
force-pushed
the
bufwrite-use-do-dialog
branch
from
e989b35
to
fe69171
Compare
|
@tjdevries you're the only core maintainer I know the name of so tagging you for feedback and/or to point me in the right direction. This is my first PR in neovim :) |
| if (vim_dialog_yesno(VIM_WARNING, | ||
| NULL, | ||
| _( | ||
| "WARNING: The file has been changed since reading it. Do you really want to write to it?"), |
There was a problem hiding this comment.
Just out of curiosity, any special reason to change the message slightly (e.g. dropping the exclamation marks)?
There was a problem hiding this comment.
No special reason, but I always found the three exclamation marks a bit over the top in addition to the all caps WARNING. Don't feel too strongly about it though.
| msgstr "¿Desea realmente escribir al archivo?" | ||
| #: ../bufwrite.c:362 | ||
| msgid "WARNING: The file has been changed since reading it. Do you really want to write to it?" | ||
| msgstr "ADVERTENCIA: El archivo ha cambiado desde que se leyó. ¿Desea realmente escribir en él?" |
There was a problem hiding this comment.
What about other locale files other than Spanish?
There was a problem hiding this comment.
I was having trouble testing this one, :lang es_ES on my dirty build doesn't seem to do anything. Do I have to put the localization files somewhere for it to pick them up?
| {9: reading it!!!} | | ||
| {6:Do you really want to write to it (y/n)?}| | ||
| ^ | | ||
| {6:WARNING: The file has been changed since}| |
There was a problem hiding this comment.
It'd be just a personal taste, but I would actually prefer shouting, intimidating warning messages that are put in red (WarningMsg) or more discernible than simple MoreMsg highlights.
There was a problem hiding this comment.
Hmm, I guess we could still print the first line in WARNING colors like it did before, but the main idea behind this change is to allow ui_attach to handle this prompt gracefully, and splitting the message into two makes it harder to do that
There was a problem hiding this comment.
I could also look into the implementation of do_dialog to see if we can use different highlights for the message
| {6:Press ENTER or type command to continue}^ | | ||
| ]]) | ||
|
|
||
| feed('<cr>') |
There was a problem hiding this comment.
With vim_dialog_yesno, after pressing 'n', the "Press ENTER" line doesn't appear
|
this part is common to vim. I suggest you submit it to vim first. |
I could give it a shot, but I suspect they won't see the value in this change since the main reason is to make it play nicely with |
In order to allow more flexibility in how UI plugins like Noice handle this specific prompt, switch from using
ask_yesnotovim_dialog_yesno, which sends amsg_send.confirmevent which can be handled byvim.ui_attach. This also brings it in line with other user prompts such as the swapfile dialog.Before:
After:
With Noice:
Questions/feedback requested:
ask_yesnoused to add). I'm happy to take a crack at that, but I'm having trouble testing it. DoingVIMRUNTIME=runtime ./build/bin/nvim --cleanand then setting:lang es_ESdoesn't seem to use Spanish strings :/