[PATCH v2] pager: do not unnecessarily set COLUMNS on Windows

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Since https://github.com/gwsw/less/commit/bb0ee4e76c2, `less` prefers
the `COLUMNS` variable over asking ncurses itself.

This is typically not a problem because we ask `TIOCGWINSZ` in Git, to
determine the correct value for `COLUMNS`, and then set that environment
variable.

However, on Windows it _is_ a problem. The reason is that Git for
Windows uses a version of `less` that relies on the MSYS2 runtime to
interact with the pseudo terminal (typically inside a MinTTY window,
which is also aware of the MSYS2 runtime). Both MinTTY and `less.exe`
interact with that pseudo terminal via `ioctl()` calls (which the MSYS2
runtime emulates even if there is no such thing on Windows).

But `git.exe` itself is _not_ aware of the MSYS2 runtime, or for that
matter of that pseudo terminal, and has no way to call `ioctl()` or
`TIOCGWINSZ`.

Therefore, `git.exe` will fall back to hard-coding 80 columns, no matter
what the actual terminal size is.

But `less.exe` is totally able to interact with the MSYS2 runtime and
would not actually require Git's help (which actually makes things
worse here). So let's not override `COLUMNS` on Windows.

Note: we do this _only_ on Windows, and _only_ if `TIOCGWINSZ` is not
defined, to reduce any potential undesired fall-out from this patch.

This fixes https://github.com/git-for-windows/git/issues/3235

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    pager: do not unnecessarily set COLUMNS on Windows
    
    A recent upgrade of the "less" package in Git for Windows causes
    problems. Here is a work-around.
    
    Changes since v1:
    
     * The commit message was reworded to clarify the underlying issue
       better.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-982%2Fdscho%2Ffix-duplicated-lines-when-moving-in-pager-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-982/dscho/fix-duplicated-lines-when-moving-in-pager-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/982

Range-diff vs v1:

 1:  22d3a9a2c9a2 ! 1:  82099e53bbc9 pager: do not unnecessarily set COLUMNS on Windows
     @@ Commit message
          the `COLUMNS` variable over asking ncurses itself.
      
          This is typically not a problem because we ask `TIOCGWINSZ` in Git, to
     -    determine the correct value for `COLUMNS`.
     +    determine the correct value for `COLUMNS`, and then set that environment
     +    variable.
      
     -    However, on Windows it _is_ a problem because Git for Windows' Bash (and
     -    `less.exe`) uses the MSYS2 runtime while `git.exe` does _not_, and
     -    therefore we never get the correct value in Git, but `less.exe` has no
     -    problem obtaining it.
     +    However, on Windows it _is_ a problem. The reason is that Git for
     +    Windows uses a version of `less` that relies on the MSYS2 runtime to
     +    interact with the pseudo terminal (typically inside a MinTTY window,
     +    which is also aware of the MSYS2 runtime). Both MinTTY and `less.exe`
     +    interact with that pseudo terminal via `ioctl()` calls (which the MSYS2
     +    runtime emulates even if there is no such thing on Windows).
      
     -    Let's not override `COLUMNS` in that case.
     +    But `git.exe` itself is _not_ aware of the MSYS2 runtime, or for that
     +    matter of that pseudo terminal, and has no way to call `ioctl()` or
     +    `TIOCGWINSZ`.
     +
     +    Therefore, `git.exe` will fall back to hard-coding 80 columns, no matter
     +    what the actual terminal size is.
     +
     +    But `less.exe` is totally able to interact with the MSYS2 runtime and
     +    would not actually require Git's help (which actually makes things
     +    worse here). So let's not override `COLUMNS` on Windows.
     +
     +    Note: we do this _only_ on Windows, and _only_ if `TIOCGWINSZ` is not
     +    defined, to reduce any potential undesired fall-out from this patch.
      
          This fixes https://github.com/git-for-windows/git/issues/3235
      


 pager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pager.c b/pager.c
index 3d37dd7adaa2..b84668eddca2 100644
--- a/pager.c
+++ b/pager.c
@@ -111,11 +111,13 @@ void setup_pager(void)
 	 * to get the terminal size. Let's grab it now, and then set $COLUMNS
 	 * to communicate it to any sub-processes.
 	 */
+#if !defined(WIN32) || defined(TIOCGWINSZ)
 	{
 		char buf[64];
 		xsnprintf(buf, sizeof(buf), "%d", term_columns());
 		setenv("COLUMNS", buf, 0);
 	}
+#endif
 
 	setenv("GIT_PAGER_IN_USE", "true", 1);
 

base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
-- 
gitgitgadget