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

fix(man.vim): hide signcolumn to avoid broken wrap #28775

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

belkka
Copy link

@belkka belkka commented May 16, 2024

This is how man page looks like when using neovim as a man pager and having signcolumn=yes. Note the line wraps in Description:

man man
Note: I did not resize the window after running the command.

Steps to reproduce:

  1. set signcolumn=yes in ~/.config/nvim/init.vim
  2. MANPAGER='nvim +Man!' man man

The suggested solution is to set signcolumn=auto in ftplugin/man.vim, because man pages are especially sensitive to terminal width. man hard-wraps content (using terminal width as a default value for MANWIDTH), not taking pager's UI into account (the default MANPAGER is less, which displays content at full width of the terminal)

ftplugin/man.vim already contains similar options (e.g. set nonumber) to address the issue. The benefit of those tweaks is lost if ANY of UI columns appear.

setlocal foldcolumn=0 colorcolumn=0 nolist nofoldenable
" man page content is likely preformatted for the terminal width, so
" narrowing display by any additional columns leads to Embarrassing Line Wrap
setlocal nonumber norelativenumber foldcolumn=0 signcolumn=auto
Copy link
Member

Choose a reason for hiding this comment

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

disabling random things is not ideal. instead, find where the width (MANWIDTH, iirc) is calculated in this plugin, and adjust it

Copy link
Author

Choose a reason for hiding this comment

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

I don't think MANWIDTH is controlled by plugin when calling man from CLI. Personally, I have MANPAGER set in my environment as described in :help man, and just call man echo in bash (see steps to reproduce).

Note, that this PR only adds signcolumn=auto (may be easier to see when viewing separate commits). Other "random things" (visual columns occupied by editor's UI, to be precise) are already disabled. Suggested changes follow the existing approach and improve it (I believe), not aiming to seek for ideal.

If someone decide to spend time finding an "ideal" solution, they could still benefit from these changes by using them as a reference. E. g. just like they will see "nonumber", "norelativenumber", "foldcolumn" and include the width of these columns into their calculations of MANWIDTH (or whatever is relevant), they will also see the "signcolumn" and remember to take it into account as well.

Also, IMHO, benefits of what you call an "ideal" approach are questionable. ftplugins are specifically intended to improve user experience with specific filetypes by taking into account the meaning of the file type. Disabling/enabling specific things for specific filetypes feels totally reasonable for me. Do you think that other setlocal options are not ideal should be avoided in this ftplugin (e. g. nolist)?

Copy link
Member

Choose a reason for hiding this comment

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

This is fine, since we already were doing nonumber norelativenumber, and :Man always opens a new window anyway (otherwise restoring window-local options is tricky).

why signcolumn=auto instead of signcolumn=no ?

Copy link
Member

@justinmk justinmk left a comment

Choose a reason for hiding this comment

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

disabling random things is not ideal. instead, find where the width (MANWIDTH, iirc) is calculated in this plugin, and adjust it

Problem:
1. multiple `setlocal` commands are spread across the script.
2. several options, apparently, serve the same purpose (hide UI columns) which may not be immediately clear. more options may be required to fullfill the same purpose or they could be removed all together as a group if better solution is found later
3. `setlocal nofoldenable` may be overriden by conditional block later in the script.

Solution:
1. move 'colorcolumn' and 'nolist' to the group of other options at the beginning
2. add an explanatory comment about options that disable UI columns
3. move 'nofoldenable' to the if-else block to keep relevant commands coupled
Problem:
It's a common practice to set 'signcolumn=yes' (always show) instead of default 'signcolumn=auto' in order to prevent annoying horizontal shifting in editable buffers when using some popular plugins that add/remove signs on the fly. This makes signcolumn always visible and breaks the text flow of pre-formatted man pages, even when no signs are actually defined. Some other options are already tweaked in man.vim to address the issue (e.g. 'nonumber'), but not signcolumn.

Solution:
set 'signcolumn=auto' in ftplugin/man.vim.
By default there is no |signs| in man pages anyway (and I am not aware of any plugins that could define them in man pages), so 'signcolumn=auto' should behave like 'signcolumn=no', i.e. hide the empty column in order to keep buffer width same as terminal width.
In a (rare?) case when user does define some signs in man pages, signcolumn will appear (breaking the text flow).
@belkka belkka force-pushed the hide-signcolumn-in-man-pages branch from 5e93cf6 to 2c160f3 Compare June 11, 2024 01:59
@belkka belkka changed the title ftplugin/man.vim: Hide signcolumn to avoid "embarrassing line wrap" fix(ftplugin/man.vim): Hide signcolumn to avoid "embarrassing line wrap" Jun 11, 2024
@belkka belkka marked this pull request as ready for review June 11, 2024 02:10
@belkka

This comment was marked as resolved.

@justinmk justinmk changed the title fix(ftplugin/man.vim): Hide signcolumn to avoid "embarrassing line wrap" fix(man.vim): hide signcolumn to avoid broken wrap Jun 12, 2024
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.

None yet

2 participants