On 26/12/11 19:22, Ben wrote:
On Dec 26, 1:16 am, Tony Mechelynck<[email protected]>
wrote:
On 26/12/11 06:37, Ben wrote:

Hi vim_dev,

I always liked the feature in win32 console vim where the cursor
changed shape depending on what mode you're in. I recently switched to
using cygwin console vim on my Windows systems instead of win32
console for various reasons (primarily because it understands cygwin
paths), and was disappointed to find the cursor shape-change feature
was not working, even though my version of cygwin vim was compiled
with +cursorshape. I had some time on my hands, so I went into the
source and figured out how to get it to work. Here's the patch:

diff -r a96cb758a8d7 runtime/doc/options.txt
--- a/runtime/doc/options.txt      Fri Dec 23 14:56:28 2011 +0100
+++ b/runtime/doc/options.txt      Mon Dec 26 00:11:47 2011 -0500
@@ -3334,17 +3334,17 @@
                                    r-cr:hor20-Cursor/lCursor,
                                    sm:block-Cursor
                                    -blinkwait175-blinkoff150-blinkon175",
-                          for MS-DOS and Win32 console:
+                          for MS-DOS, Win32, and Cygwin console:
                                    "n-v-c:block,o:hor50,i-ci:hor15,
                                    r-cr:hor30,sm:block")
                    global
                    {not in Vi}
                    {only available when compiled with GUI enabled, and
-                  for MS-DOS and Win32 console}
+                  for MS-DOS, Win32, and Cygwin console}
    This option tells Vim what the cursor should look like in different
-  modes.  It fully works in the GUI.  In an MSDOS or Win32 console,
only
-  the height of the cursor can be changed.  This can be done by
-  specifying a block cursor, or a percentage for a vertical or
+  modes.  It fully works in the GUI.  In an MSDOS, Win32, and Cygwin
+  console, only the height of the cursor can be changed.  This can be
+  done by specifying a block cursor, or a percentage for a vertical or
    horizontal cursor.
    For a console the 't_SI' and 't_EI' escape sequences are used.

diff -r a96cb758a8d7 src/feature.h
--- a/src/feature.h        Fri Dec 23 14:56:28 2011 +0100
+++ b/src/feature.h        Mon Dec 26 00:11:47 2011 -0500
@@ -1156,8 +1156,8 @@
    * mouse shape           Adjust the shape of the mouse pointer to the mode.
    */
   #ifdef FEAT_NORMAL
-/* MS-DOS console and Win32 console can change cursor shape */
-# if defined(MSDOS) || (defined(WIN3264)&&    !defined(FEAT_GUI_W32))
+/* MS-DOS console, Win32 console, and Cygwin console can change
cursor shape */
+# if defined(MSDOS) || (defined(WIN3264)&&    !defined(FEAT_GUI_W32))
|| defined(__CYGWIN__)
   #  define MCH_CURSOR_SHAPE
   # endif
   # if defined(FEAT_GUI_W32) || defined(FEAT_GUI_W16) ||
defined(FEAT_GUI_MOTIF) \
diff -r a96cb758a8d7 src/os_unix.c
--- a/src/os_unix.c        Fri Dec 23 14:56:28 2011 +0100
+++ b/src/os_unix.c        Mon Dec 26 00:11:47 2011 -0500
@@ -31,6 +31,11 @@

   #include "vim.h"

+#if defined(__CYGWIN__)
+#include<windows.h>
+static HANDLE g_hConOut = INVALID_HANDLE_VALUE;
+#endif
+
   #ifdef FEAT_MZSCHEME
   # include "if_mzsch.h"
   #endif
@@ -1222,6 +1227,10 @@
   #ifdef MACOS_CONVERT
       mac_conv_init();
   #endif
+
+#if defined(__CYGWIN__)
+    g_hConOut = GetStdHandle(STD_OUTPUT_HANDLE);
+#endif
   }

       static void
@@ -7196,3 +7205,41 @@

   #endif
+
+#if defined(__CYGWIN__)
+
+#if defined(MCH_CURSOR_SHAPE)
+/*
+ * Set the shape of the cursor.
+ * 'thickness' can be from 1 (thin) to 99 (block)
+ */
+    static void
+mch_set_cursor_shape(int thickness)
+{
+    CONSOLE_CURSOR_INFO ConsoleCursorInfo;
+    ConsoleCursorInfo.dwSize = thickness;
+    ConsoleCursorInfo.bVisible = TRUE;
+
+    SetConsoleCursorInfo(g_hConOut,&ConsoleCursorInfo);
+}
+
+    void
+mch_update_cursor(void)
+{
+    int           idx;
+    int           thickness;
+
+    /*
+     * How the cursor is drawn depends on the current mode.
+     */
+    idx = get_shape_idx(FALSE);
+
+    if (shape_table[idx].shape == SHAPE_BLOCK)
+  thickness = 99; /* 100 doesn't work on W95 */
+    else
+  thickness = shape_table[idx].percentage;
+    mch_set_cursor_shape(thickness);
+}
+#endif
+
+#endif

Some possible concerns:
1. I know that cygwin vim can run in different terminals, such as rxvt
and the new cygwin terminal. The patch obviously only works when run
in a DOS box because it calls the Windows API function
SetConsoleCursorInfo, which only applies to the DOS box. I'm not sure
how to test what type of console we're running in... should we include
such a test to avoid calling that code unnecessarily?

Isn't that what the 'term' option is for? My vimrc includes the following:

      if (&term == "pcterm") || (&term == "win32")
          " if exists("+guicursor")
          " Console cursor shape (Windows only)
          set guicursor=n-v-c:block,o:hor50,i-ci:hor15,r-cr:hor30
          set guicursor+=sm:block,a:blinkwait750-blinkoff750-blinkon750
      elseif (...)

What is 'term' set to in a "classical" cygwin bash terminal (like I used
to have on XP SP2, when that was still state-of-the-art)? Can you call
SetConsoleCursorInfo() there?

Ah, you're right, we can test the&term option from the code. I'm not
sure what you mean by the "classical" cygwin bash terminal, I assume
you mean the DOS box that cygwin bash runs in by default? The value of
&term in that case is "cygwin", and that's the only case in which a
call to SetConsoleCursorInfo() will work, because Windows API console
functions only applies to the DOS box. So I've added some code to
os_unix.c to perform that test. Here's the updated part of the patch:

Well, when I still had a Windows box some time ago, when the latest Windows OS was XP SP2, I had Cygwin on it, with only one kind of Unix-like environment, namely a terminal running Cygwin bash, and I never knew if cmd.exe was "under" it or not. My "usual" cmd.exe box had nonstandard colours (bright cyan on blue), my Cygwin bash box had the default light grey on black, and so they were easy to tell apart. And yes, that's the bash terminal that I meant in my earlier post. I suppose the "new Cygwin terminal" didn't yet exist back then?

Just make sure you don't test &term before it's set (from $TERM etc.): this is a case when the order of initializations counts.


+
+    void
+mch_update_cursor(void)
+{
+    int                idx;
+    int                thickness;
+
+    /*
+     * It's possible to run cygwin vim from various different
terminals,
+     * however, the cursor shape code only works in the DOS box, i.e.
when
+     *&term == "cygwin". This is because it uses the Windows API
function
+     * SetConsoleCursorInfo() to set the cursor shape, which only
applies
+     * to the DOS box.
+     */
+    char_u* term = 0;
+    get_option_value((char_u *)"term", 0,&term, 0 );
+    if (STRCMP(term, (char_u *)"cygwin" ) != 0)
+        return;
+
+    /*
+     * How the cursor is drawn depends on the current mode.
+     */
+    idx = get_shape_idx(FALSE);
+

2. I believe cygwin vim can be compiled for GUI mode, so the code
wouldn't work in that case either; should we include a preprocessor
test to exclude the code if compiling for GUI mode?

In GUI mode you should set 'guicursor' the normal way (allowing vertical
bar in addition to horizontal bar or block), shouldn't you?

I'm not sure what you mean by setting 'guicursor' the normal way; do
you mean the cursor is already updated correctly in GUI mode? I
haven't tried the GUI in cygwin, so I'm not sure. Doesn't that mean we
should include a preprocessor condition to exclude the new code for
GUI mode?

I mean that in all flavours of gvim, including Cygwin-X11 GUI, the 'guicursor' option defines a graphical cursor which can be either a full block, a horizontal bar of selectable height near the bottom, or a vertical bar of selectable width near the left. I would assume that this already works. And yes, you should disable the console-cursor code (if it isn't yet) when running in GUI mode, either at run-time (if a single Cygwin executable can run in both Console mode or GUI mode, which I don't expect), or at compile-time (otherwise). Check first if it isn't already excluded by an outer #ifdef or by not compiling that module in a GUI build.

Also, IIRC, during gvim startup (when it's known that we're going to start gvim, has('gui_running') returns true, but we haven't yet (e.g. we're still sourcing the vimrc), setting 'guicursor' to values typical of the GUI gives no error even if the new values are invalid in console mode, they are remembered, and applied when starting the GUI (about when the GUIEnter event is triggered).


3. I'm not sure about how much testing needs to be done for a new
patch (this is the first patch I've ever submitted); I've only tested
it on one Windows 7 machine.

I suppose that it should be tested as extensively as possible, to ensure that: - it works in all Cygwin versions you could lay hands on, in as many Windows versions as possible; - it doesn't introduce new bugs in non-Cygwin Windows builds (which you can test) or in non-Windows builds (meaning Linux and Mac OSX at the very least, others if possible; here I suppose other people would have to do the testing)

And of course Bram has the last word on which patches are judged stable enough to be included in the official Vim code.


Any feedback is appreciated.

Thanks,
Ben

Best regards,
Tony.
--
Q:  How many IBM cpu's does it take to do a logical right shift?
A:  33.  1 to hold the bits and 32 to push the register.

Thanks,
Ben


Best regards,
Tony.
--
Hand, n.:
        A singular instrument worn at the end of a human arm and
commonly thrust into somebody's pocket.
                -- Ambrose Bierce, "The Devil's Dictionary"

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Raspunde prin e-mail lui