Skip to content

Conversation

mhinz
Copy link
Member

@mhinz mhinz commented Jul 11, 2018

Show a proper confirmation dialog when trying to unload a terminal buffer while
the confirm option is set or when :confirm is used.

Fixes #4651

@KillTheMule
Copy link
Contributor

Can you dump the screen in an array of strings and assert each one individually? For my plugin I've modified Screen:expect to not compare certain lines, maybe we need to make that more flexible so uses such as this are covered.

bool dialog_unload_terminal(buf_T *buf) {
char_u buff[DIALOG_MSG_SIZE];

dialog_msg(buff, _("Unload \"%s\"?"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we clarify "Kill and unload", or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe "Closing", since that is a term we often use with terminals? Like TermClose etc.?

@marvim marvim added the RFC label Jul 11, 2018
@mhinz
Copy link
Member Author

mhinz commented Jul 11, 2018

@KillTheMule The channel taught me about

-- any: true: Succeed if `expected` matches ANY screen line(s).
:)

I added screen tests.

@mhinz mhinz force-pushed the terminal/confirmation branch 2 times, most recently from 7180d7c to 17117ca Compare July 11, 2018 16:43
Show a proper confirmation dialog when trying to unload a terminal buffer while
the confirm option is set or when :confirm is used.

Fixes neovim#4651
@mhinz mhinz force-pushed the terminal/confirmation branch from 17117ca to 6dd6ff1 Compare July 11, 2018 17:02
@KillTheMule
Copy link
Contributor

KillTheMule commented Jul 11, 2018

Yeah I knew about that, but that's not what one wants most of the time, right? I mean, usually in tests there are some blank lines which makes this kind of assert kind of redundant.

(e) Oh wait, hahaha, I misunderstood, good one!

char_u buff[DIALOG_MSG_SIZE];

dialog_msg(buff, _("Close \"%s\"?"),
(buf->b_fname != NULL) ? buf->b_fname : (char_u *)_("Untitled"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just untranslated "?" is fine instead of "Untitled" (which is not common in vim, more like MS Word).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took that directly from dialog_changed():

neovim/src/nvim/ex_cmds2.c

Lines 1288 to 1290 in 9adb6ed

dialog_msg(buff, _("Save changes to \"%s\"?"),
(buf->b_fname != NULL) ?
buf->b_fname : (char_u *)_("Untitled"));

But I agree with the "?". It probably never happens that the current terminal buffer's name is NULL.

EMSG2(_("E89: %s will be killed(add ! to override)"),
(char *)buf->b_fname);
if (p_confirm || cmdmod.confirm) {
if (dialog_close_terminal(buf) == false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't usually compare booleans explicitly like this, it's ok to use !.

@mhinz mhinz merged commit 01570f1 into neovim:master Jul 12, 2018
@mhinz mhinz deleted the terminal/confirmation branch July 12, 2018 12:57
@justinmk justinmk removed the RFC label Jul 12, 2018
justinmk added a commit that referenced this pull request Jul 18, 2018
FEATURES:
07499a8 #8709 man.vim: C highlighting for EXAMPLES section
07f82ad #8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 #8616 API: emit nvim_buf_lines_event from :terminal
c46997a #8546 fillchars: Add "eob" flag

FIXES:
74d19f6 #8576 startup: avoid blank stdin buffer if other files were opened
4874214 #8737 Only waitpid() for processes that we care about
cd6e7e8 #8743 Check all child processes for exit in SIGCHLD handler
c230ef2 #8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 #8681 transstr_buf: fix length comparison
d241f27 #8708 TUI: Fix standout mode
9afed40 #8698 man.vim: fix for mandoc
e889640 #8682 provider/node: npm --loglevel silent
1cbc830 #8613 API: nvim_win_set_cursor: set curswant
bf6048e #8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 #8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 #8619 defaults: shortmess+=F
1248178 #8578 highlight: high-priority CursorLine if fg is set.
01570f1 #8726 terminal: handle &confirm and :confirm on unloading
56065bb #8721 screen: truncate showmode messages
bf2460e #7551 buffer: fix copying :setlocal options
c1c14fa #8520 Ex mode: always "improved" (gQ)
050f397 #7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 #7992 screen: use UTF-8 representation
coditva pushed a commit to coditva/neovim that referenced this pull request Jul 28, 2018
Show a proper confirmation dialog when trying to unload a terminal buffer while
the confirm option is set or when :confirm is used.

Fixes neovim#4651
coditva pushed a commit to coditva/neovim that referenced this pull request Jul 28, 2018
FEATURES:
07499a8 neovim#8709 man.vim: C highlighting for EXAMPLES section
07f82ad neovim#8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 neovim#8616 API: emit nvim_buf_lines_event from :terminal
c46997a neovim#8546 fillchars: Add "eob" flag

FIXES:
74d19f6 neovim#8576 startup: avoid blank stdin buffer if other files were opened
4874214 neovim#8737 Only waitpid() for processes that we care about
cd6e7e8 neovim#8743 Check all child processes for exit in SIGCHLD handler
c230ef2 neovim#8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 neovim#8681 transstr_buf: fix length comparison
d241f27 neovim#8708 TUI: Fix standout mode
9afed40 neovim#8698 man.vim: fix for mandoc
e889640 neovim#8682 provider/node: npm --loglevel silent
1cbc830 neovim#8613 API: nvim_win_set_cursor: set curswant
bf6048e neovim#8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 neovim#8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 neovim#8619 defaults: shortmess+=F
1248178 neovim#8578 highlight: high-priority CursorLine if fg is set.
01570f1 neovim#8726 terminal: handle &confirm and :confirm on unloading
56065bb neovim#8721 screen: truncate showmode messages
bf2460e neovim#7551 buffer: fix copying :setlocal options
c1c14fa neovim#8520 Ex mode: always "improved" (gQ)
050f397 neovim#7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 neovim#7992 screen: use UTF-8 representation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants