Skip to content

Conversation

glts
Copy link
Contributor

@glts glts commented May 9, 2015

Problem:    Having CTRL-C interrupt or not does not check the mode of the
        mapping. (Ingo Karkat)
Solution:   Use a bitmask with the map mode. (Christian Brabandt)

vim/vim@v7-4-569

Original patch:

diff --git a/src/nvim/getchar.c b/src/nvim/getchar.c
--- a/src/nvim/getchar.c
+++ b/src/nvim/getchar.c
@@ -3708,8 +3708,13 @@ do_map(maptype, arg, mode, abbrev)
    if (!did_it)
        retval = 2;             /* no match */
    else if (*keys == Ctrl_C)
+   {
        /* If CTRL-C has been unmapped, reuse it for Interrupting. */
-       mapped_ctrl_c = FALSE;
+       if (map_table == curbuf->b_maphash)
+       curbuf->b_mapped_ctrl_c &= ~mode;
+       else
+       mapped_ctrl_c &= ~mode;
+   }
    goto theend;
     }

@@ -3744,7 +3749,12 @@ do_map(maptype, arg, mode, abbrev)

     /* If CTRL-C has been mapped, don't always use it for Interrupting. */
     if (*keys == Ctrl_C)
-   mapped_ctrl_c = TRUE;
+    {
+   if (map_table == curbuf->b_maphash)
+       curbuf->b_mapped_ctrl_c |= mode;
+   else
+       mapped_ctrl_c |= mode;
+    }

     mp->m_keys = vim_strsave(keys);
     mp->m_str = vim_strsave(rhs);
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -958,7 +958,7 @@ EXTERN char_u   *exe_name;      /* the name of
 #ifdef USE_ON_FLY_SCROLL
 EXTERN int dont_scroll INIT(= FALSE);/* don't use scrollbars when TRUE */
 #endif
-EXTERN int mapped_ctrl_c INIT(= FALSE); /* CTRL-C is mapped */
+EXTERN int mapped_ctrl_c INIT(= FALSE); /* modes where CTRL-C is mapped */
 EXTERN int ctrl_c_interrupts INIT(= TRUE); /* CTRL-C sets got_int */

 EXTERN cmdmod_T    cmdmod;         /* Ex command modifiers */
diff --git a/src/nvim/structs.h b/src/nvim/structs.h
--- a/src/nvim/structs.h
+++ b/src/nvim/structs.h
@@ -1802,6 +1802,7 @@ struct file_buffer
     cryptstate_T *b_cryptstate;    /* Encryption state while reading or writing
                 * the file. NULL when not using encryption. */
 #endif
+    int        b_mapped_ctrl_c; /* modes where CTRL-C is mapped */

 }; /* file_buffer */

diff --git a/src/nvim/testdir/test_mapping.in b/src/nvim/testdir/test_mapping.in
--- a/src/nvim/testdir/test_mapping.in
+++ b/src/nvim/testdir/test_mapping.in
@@ -8,6 +8,15 @@ STARTTEST
 :inoreab чкпр   vim
 GAчкпр 
 �
+:" mapping of ctrl-c in insert mode
+:set cpo-=< cpo-=k
+:inoremap <c-c> <ctrl-c>
+:cnoremap <c-c> dummy
+:cunmap <c-c>
+GA
+TEST2: CTRL-C |�A|
+
+:nunmap <c-c>

 : " langmap should not get remapped in insert mode
 :inoremap { FAIL_ilangmap
diff --git a/src/nvim/testdir/test_mapping.ok b/src/nvim/testdir/test_mapping.ok
--- a/src/nvim/testdir/test_mapping.ok
+++ b/src/nvim/testdir/test_mapping.ok
@@ -1,4 +1,6 @@
 test starts here:
 vim
+TEST2: CTRL-C |<ctrl-c>A|
+
 +
 +
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -180,7 +180,7 @@ ui_inchar(buf, maxlen, wtime, tb_change_

    /* ... there is no need for CTRL-C to interrupt something, don't let
     * it set got_int when it was mapped. */
-   if (mapped_ctrl_c)
+   if ((mapped_ctrl_c | curbuf->b_mapped_ctrl_c) & State)
        ctrl_c_interrupts = FALSE;
     }

diff --git a/src/nvim/version.c b/src/nvim/version.c
--- a/src/nvim/version.c
+++ b/src/nvim/version.c
@@ -742,6 +742,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    569,
+/**/
     568,
 /**/
     567,

Problem:    Mapping CTRL-C in Visual mode doesn't work. (Ingo Karkat)
Solution:   Call get_real_state() instead of using State directly.

vim/vim@v7-4-573

Original patch:

diff --git a/src/nvim/testdir/test_mapping.in b/src/nvim/testdir/test_mapping.in
--- a/src/nvim/testdir/test_mapping.in
+++ b/src/nvim/testdir/test_mapping.in
@@ -8,7 +8,7 @@ STARTTEST
 :inoreab чкпр   vim
 GAчкпр 
 �
-:" mapping of ctrl-c in insert mode
+:" mapping of ctrl-c in Insert mode
 :set cpo-=< cpo-=k
 :inoremap <c-c> <ctrl-c>
 :cnoremap <c-c> dummy
@@ -16,9 +16,15 @@ GAчкпр
 GA
 TEST2: CTRL-C |�A|
 �
-:nunmap <c-c>
-
-: " langmap should not get remapped in insert mode
+:unmap <c-c>
+:unmap! <c-c>
+:"
+:" mapping of ctrl-c in Visual mode
+:vnoremap <c-c> :<C-u>$put ='vmap works'
+GV�
+:vunmap <c-c>
+:"
+:" langmap should not get remapped in insert mode
 :inoremap { FAIL_ilangmap
 :set langmap=+{ langnoremap
 o+�
diff --git a/src/nvim/testdir/test_mapping.ok b/src/nvim/testdir/test_mapping.ok
--- a/src/nvim/testdir/test_mapping.ok
+++ b/src/nvim/testdir/test_mapping.ok
@@ -2,5 +2,6 @@ test starts here:
 vim
 TEST2: CTRL-C |<ctrl-c>A|

+vmap works
 +
 +
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -180,7 +180,7 @@ ui_inchar(buf, maxlen, wtime, tb_change_

    /* ... there is no need for CTRL-C to interrupt something, don't let
     * it set got_int when it was mapped. */
-   if ((mapped_ctrl_c | curbuf->b_mapped_ctrl_c) & State)
+   if ((mapped_ctrl_c | curbuf->b_mapped_ctrl_c) & get_real_state())
        ctrl_c_interrupts = FALSE;
     }

diff --git a/src/nvim/version.c b/src/nvim/version.c
--- a/src/nvim/version.c
+++ b/src/nvim/version.c
@@ -742,6 +742,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    573,
+/**/
     572,
 /**/
     571,

@marvim marvim added the WIP label May 9, 2015
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before, mapped_ctrl_c was a Boolean value, where true indicated that Ctrl-C was mapped and false that it wasn't mapped. Now that mapped_ctrl_c is an int with mode flags, the true case becomes 'all flags turned on', which I think can be expressed as MAP_ALL_MODES. Correct?

(Why is Ctrl-C always 'mapped' when entering the terminal though? I don't understand this.)

@glts glts changed the title [WIP] vim-patch:7.4.{569,573} [RFC] vim-patch:7.4.{569,573} May 9, 2015
@marvim marvim added RFC and removed WIP labels May 9, 2015
@glts
Copy link
Contributor Author

glts commented May 9, 2015

Test failure.

It passed locally but now I see there is some indeterminism in this test. Look at the failing test results, they're not all the same. Hmm.

@glts glts changed the title [RFC] vim-patch:7.4.{569,573} [WIP] vim-patch:7.4.{569,573} May 9, 2015
@marvim marvim added WIP and removed RFC labels May 9, 2015
@glts
Copy link
Contributor Author

glts commented May 10, 2015

Still failing! Frustrating. Does anybody know why these tests fail and how to make them pass in the CI? It's quite bad with these brittle functional tests at the moment. Is this a fundamental problem in the test infrastructure?

@jszakmeister
Copy link
Contributor

It looks like the quickbuild-related issue (with the functional tests timing out) could be fixed if you rebased on master. The tests were hanging on QB because a necessary executable was not being built, and the tests didn't really do well picking up on that.

I wish I knew about the Travis ones. I know we've got a couple that are a bit racy, but I'm not sure what's happening here. :-(

@glts
Copy link
Contributor Author

glts commented May 10, 2015

blimey the test passed on Travis 🍸 No idea why.

Now on to clean up the mess I made.

@glts glts changed the title [WIP] vim-patch:7.4.{569,573} [RFC] vim-patch:7.4.{569,573} May 10, 2015
@marvim marvim added RFC and removed WIP labels May 10, 2015
glts added 2 commits May 26, 2015 19:51
Problem:    Having CTRL-C interrupt or not does not check the mode of the
            mapping. (Ingo Karkat)
Solution:   Use a bitmask with the map mode. (Christian Brabandt)

vim/vim@v7-4-569
Problem:    Mapping CTRL-C in Visual mode doesn't work. (Ingo Karkat)
Solution:   Call get_real_state() instead of using State directly.

vim/vim@v7-4-573

The code change in src/ui.c was already applied in src/nvim/os/input.c
in patch 7.4.569.

Also split monolithic test into four "it" blocks because of persistent
non-deterministic test failures on CI.
@glts
Copy link
Contributor Author

glts commented May 26, 2015

Rebased, no changes.

@felipecrv
Copy link
Contributor

LGTM.

@justinmk
Copy link
Member

restarted build (in case anyone missed it: travis infrastructure problems appear to be the cause of the many build failures in May; they have been running smoothly since then)

@Shougo
Copy link
Contributor

Shougo commented Dec 19, 2015

@glts Can you fix the conflicts?

Shougo added a commit to Shougo/neovim that referenced this pull request Dec 19, 2015
Problem:    Having CTRL-C interrupt or not does not check the mode of the
            mapping. (Ingo Karkat)
Solution:   Use a bitmask with the map mode. (Christian Brabandt)

vim/vim@651863c

Problem:    Mapping CTRL-C in Visual mode doesn't work. (Ingo Karkat)
Solution:   Call get_real_state() instead of using State directly.

vim/vim@5000869
@Shougo
Copy link
Contributor

Shougo commented Dec 19, 2015

@glts I fixed the conflicts in #3866.
Thanks.

@justinmk justinmk closed this Dec 26, 2015
@justinmk justinmk removed the RFC label Dec 26, 2015
Shougo added a commit to Shougo/neovim that referenced this pull request Jan 9, 2016
Problem:    Having CTRL-C interrupt or not does not check the mode of the
            mapping. (Ingo Karkat)
Solution:   Use a bitmask with the map mode. (Christian Brabandt)

vim/vim@651863c

Problem:    Mapping CTRL-C in Visual mode doesn't work. (Ingo Karkat)
Solution:   Call get_real_state() instead of using State directly.

vim/vim@5000869
justinmk pushed a commit to justinmk/neovim that referenced this pull request Jan 13, 2016
vim-patch:7.4.569
vim-patch:7.4.573
Helped-by: @glts neovim#2621

Problem:    Having CTRL-C interrupt or not does not check the mode of the
            mapping. (Ingo Karkat)
Solution:   Use a bitmask with the map mode. (Christian Brabandt)

vim/vim@651863c

Problem:    Mapping CTRL-C in Visual mode doesn't work. (Ingo Karkat)
Solution:   Call get_real_state() instead of using State directly.

vim/vim@5000869
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.

6 participants