Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:Git <any-command> --help mishandles formatting #2300

Closed
finite-state-machine opened this issue May 9, 2024 · 7 comments
Closed

:Git <any-command> --help mishandles formatting #2300

finite-state-machine opened this issue May 9, 2024 · 7 comments

Comments

@finite-state-machine
Copy link

finite-state-machine commented May 9, 2024

Running e.g. :Git commit --help displays the relevant man page in a new window (as expected), but various formatting is mishandled, as shown in the following screenshot:
Screenshot 2024-05-09 at 19 27 43

IMO it would be more helpful to show a correctly formatted man page, but it would also be reasonable and preferable to suspend VIm and run the man process in the foreground.

My quick read [edit: confirmed by an answer on StackOverflow]: the manpage viewer is using ^H (backspace) to double-strike some characters for emphasis [edit: and overlay underscores with characters to simulate underlining] as might make sense on e.g. a dot matrix printer. For example, the sequence:

D^HDE^HES^HSC^HCR^HRI^HIP^HPT^HTI^HIO^HON^HN

...prints D, moves the carriage back one character, then prints DE, and so on. This ultimately renders the word DESCRIPTION by double-striking each non-control character. The slight misalignment between the first strike and the second (no printer is that precise) would result in a bold rendition even on a plain-text printer.

Debugging observations and platform details
  • VIm recognizes the document as a man page (set ft?ft=man)
  • I'm running on macOS 14 ("Sonoma") with Fugitive 0f693bf
  • man and less are supplied by the OS (at least, when invoked from the command-line)
    • man seems to have no version-reporting facility
    • less reports its version (omitting the warranty statement) as
      less 643 (PCRE2 regular expressions)
      Copyright (C) 1984-2023  Mark Nudelman
      
  • VIm is installed via homebrew:
    VIM - Vi IMproved 9.1 (2024 Jan 02, compiled Apr 18 2024 20:05:12)
    macOS version - arm64
    Included patches: 1-350
    Compiled by Homebrew
    Huge version without GUI.  Features included (+) or not (-):
    +acl               +file_in_path      +mouse_urxvt       -tag_any_white
    +arabic            +find_in_path      +mouse_xterm       -tcl
    +autocmd           +float             +multi_byte        +termguicolors
    +autochdir         +folding           +multi_lang        +terminal
    -autoservername    -footer            -mzscheme          +terminfo
    -balloon_eval      +fork()            +netbeans_intg     +termresponse
    +balloon_eval_term +gettext           +num64             +textobjects
    -browse            -hangul_input      +packages          +textprop
    ++builtin_terms    +iconv             +path_extra        +timers
    +byte_offset       +insert_expand     +perl              +title
    +channel           +ipv6              +persistent_undo   -toolbar
    +cindent           +job               +popupwin          +user_commands
    -clientserver      +jumplist          +postscript        +vartabs
    +clipboard         +keymap            +printer           +vertsplit
    +cmdline_compl     +lambda            +profile           +vim9script
    +cmdline_hist      +langmap           -python            +viminfo
    +cmdline_info      +libcall           +python3           +virtualedit
    +comments          +linebreak         +quickfix          +visual
    +conceal           +lispindent        +reltime           +visualextra
    +cryptv            +listcmds          +rightleft         +vreplace
    +cscope            +localmap          +ruby              +wildignore
    +cursorbind        +lua               +scrollbind        +wildmenu
    +cursorshape       +menu              +signs             +windows
    +dialog_con        +mksession         +smartindent       +writebackup
    +diff              +modify_fname      +sodium            -X11
    +digraphs          +mouse             +sound             -xattr
    -dnd               -mouseshape        +spell             -xfontset
    -ebcdic            +mouse_dec         +startuptime       -xim
    +emacs_tags        -mouse_gpm         +statusline        -xpm
    +eval              -mouse_jsbterm     -sun_workshop      -xsmp
    +ex_extra          +mouse_netterm     +syntax            -xterm_clipboard
    +extra_search      +mouse_sgr         +tag_binary        -xterm_save
    -farsi             -mouse_sysmouse    -tag_old_static
       system vimrc file: "$VIM/vimrc"
         user vimrc file: "$HOME/.vimrc"
     2nd user vimrc file: "~/.vim/vimrc"
     3rd user vimrc file: "~/.config/vim/vimrc"
          user exrc file: "$HOME/.exrc"
           defaults file: "$VIMRUNTIME/defaults.vim"
      fall-back for $VIM: "/opt/homebrew/share/vim"
    Compilation: clang -c -I. -Iproto -DHAVE_CONFIG_H -DMACOS_X -DMACOS_X_DARWIN -g -O2 -D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
    Linking: clang -o vim -lm -lncurses -lsodium -liconv -lintl -framework AppKit -L/opt/homebrew/opt/lua/lib -llua5.4 -mmacosx-version-min=14.2 -fstack-protector-strong -L/opt/homebrew/opt/perl/lib/perl5/5.38/darwin-thread-multi-2level/CORE -lperl -L/opt/homebrew/opt/python@3.12/Frameworks/Python.framework/Versions/3.12/lib/python3.12/config-3.12-darwin -lpython3.12 -framework CoreFoundation -lruby.3.3 -L/opt/homebrew/Cellar/ruby/3.3.0/lib
    
@finite-state-machine
Copy link
Author

Relevant, and perhaps helpful: https://stackoverflow.com/questions/60106514/

@tpope
Copy link
Owner

tpope commented May 10, 2024

This works fine on Linux, and I could have sworn it used to work fine on macOS. I wonder if something changed.

https://unix.stackexchange.com/questions/13267/how-to-disable-styling-on-man-pages demonstrates the behavior I expect: ASCII style formatting is disabled when TERM=dumb. Making Fugitive responsible for cleaning it up is inconvenient, to say the least.

@finite-state-machine
Copy link
Author

I agree it doesn't make sense for Fugitive to mess around with control bytes.

I can see three possible fixes:

  • [always] set TERM=dumb or pipe through col -b, if this doesn't cause other problems
  • [always] wrap man in a shell script, put the wrapper in a temp dir, and put the temp dir at the front of the $PATH when invoking git
  • [conditional] detect --help (but presumably not after --) and conditionally use TERM or col

Obviously some of this doesn't make sense on Windows or other non-POSIXy platforms.

The wrapper script might look like this:

#!/usr/bin/env sh

TERM=dumb /path/to/real/man $@

(My shell script is rusty; take this with a grain of salt.)

I'm willing to help in principle, but I'm not experienced with VIm scripting.

@tpope
Copy link
Owner

tpope commented May 11, 2024

I agree it doesn't make sense for Fugitive to mess around with control bytes.

I can see three possible fixes:

  • [always] set TERM=dumb or pipe through col -b, if this doesn't cause other problems

We do always set TERM=dumb. The problem is that on macOS, TERM=dumb is ignored.

Always setting col -b feels like a bad idea. For other commands, I don't want to be perpetually unsure if the output is mangled or not.

  • [always] wrap man in a shell script, put the wrapper in a temp dir, and put the temp dir at the front of the $PATH when invoking git

This is probably the least worst option. Inconvenient but not impossible. There's also man.man.path that could be pointed at the wrapper script to avoid getting PATH involved.

  • [conditional] detect --help (but presumably not after --) and conditionally use TERM or col

We already detect --help, so that part's fine. But conditionally using col means we can't just directly execute git, we need to involve the shell. This is more disruptive than the last one.

Obviously some of this doesn't make sense on Windows or other non-POSIXy platforms.

This complicates the implementation, and is part of the "inconvenient", but not a deal breaker.

The wrapper script might look like this:

#!/usr/bin/env sh

TERM=dumb /path/to/real/man $@

What is /path/to/real/man here? If there's another man we could call that does the right thing, we could do away with the wrapper script entirely, and it gets a lot less annoying.

@finite-state-machine
Copy link
Author

What is /path/to/real/man here? If there's another man we could call that does the right thing, we could do away with the wrapper script entirely, and it gets a lot less annoying.

The idea of using a separate script was that we could set TERM (or pipe man output through col -b) without impacting output from git itself – if git doesn't invoke man, the script is never executed and nothing changes.

If you can pass git a config setting (you mentioned man.man.path) to invoke a wrapper script by path, there's no need to "spell out" the path to man in the script, in which case the 2nd line would become TERM=dumb man $@ rather than TERM=dumb /path/to/man $@.

  • If you need to find /path/to/man (i.e., the path to the system's man) – and it sounds like you don't – you'd probably choose to capture the output of which man prior to altering the $PATH to include the directory enclosing the wrapper.

Taking all of that into account, it sounds like the best approach might be (pseudo-code):

$script_path := new_temporary_file_path()
write this literal to $script_path:
    #!/usr/bin/env sh
    TERM=dumb man $@ | col -b  # presumably, on any given platform, either `TERM` or `col -b` will do the trick
when it's time to invoke git:
    run "git -c man.man.path=" + shell_escape($script_path) + "  <usual git arguments>"

@tpope tpope closed this as completed in 10fb874 May 17, 2024
@tpope
Copy link
Owner

tpope commented May 17, 2024

Looks like man uses MANPAGER even when not connected to a terminal, which isn't what I would expect, but works to our advantage as a straightforward solution.

@finite-state-machine
Copy link
Author

That works beautifully! Thank you, @tpope!

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

No branches or pull requests

2 participants