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

Width can be an offset from the terminal width #727

Merged
merged 1 commit into from Oct 18, 2021

Conversation

th1000s
Copy link
Contributor

@th1000s th1000s commented Oct 2, 2021

No longer subtract 1 from the terminal width to accommodate e.g.
less --status-columns, instead set --width -1 explicitly.


Using line wrapping combined with ANSI codes to extend the terminal width requires exact knowledge of the painted area, so make reductions of it more explicit.

--width 123-4 is also possible in to make calling with e.g. $myCOLS-2 possible.

src/options/set.rs Outdated Show resolved Hide resolved
src/options/set.rs Outdated Show resolved Hide resolved
@th1000s
Copy link
Contributor Author

th1000s commented Oct 4, 2021

Done and done! I hope the code to produce precise error messages for wrong arguments isn't too much :)

@th1000s
Copy link
Contributor Author

th1000s commented Oct 4, 2021

This PR would undo the implicit fixes for less -J / --status-column (see #10 and #41,
@igrekster @fdcds @nedbat / edit: also #115 @gibfahn which would require -2) and require those users to set width = -1. Do any distributions ship with these less options out-of-the-box? If so maybe autodetection is required.

src/cli.rs Outdated
/// full terminal width.
/// The width of underline/overline decorations. Can be absolute, absolute [+|-] offset, or
/// only an offset - in the last case the detected terminal width is changed
/// by the offset. Examples: "72", "80-2", "-2" or "98+2".
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remind me what a common use case for + is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, none that I can think of - feature completeness? Making sure the ALU still knows how to add?

Copy link
Owner

@dandavison dandavison Oct 4, 2021

Choose a reason for hiding this comment

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

Haha :) okay, I'm just thinking about how to make it as easy as possible to understand. What do you think about something like

The width of underline/overline decorations. Examples: "72" (exactly 72 characters), "-2" (auto-detected terminal width minus 2). An expression such as "74-2" is also valid (equivalent to 72 but may be useful in shell programming if a variable exists holding the value "74").

@dandavison
Copy link
Owner

@th1000s this is marked "Draft" -- is that accurate, or is it ready to merge?

@th1000s
Copy link
Contributor Author

th1000s commented Oct 14, 2021

This commit is part of the work to restore ANSI sequences in side-by-side mode. I pushed it early because it changes the UI a bit and wanted to give it some time for feedback. Lets wait for the other PR before merging this :)

No longer subtract 1 from the terminal width to accommodate e.g.
`less --status-columns`, instead set `--width -1` explicitly.
@th1000s th1000s marked this pull request as ready for review October 17, 2021 19:06
Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

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

LGTM!

@dandavison dandavison merged commit 07228cc into dandavison:master Oct 18, 2021
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