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

CLI --help font or character format trouble when PAGER=less and LESS is unset (Alpine/Gentoo/NixOS default) #4745

Open
andar1an opened this issue Oct 31, 2024 · 39 comments

Comments

@andar1an
Copy link

Description

When launching the jj help on cli, the help doesn't appear correct - seems like it has character representations, but I can't determine if these are ascii, utf, or unicode character representation, it doesn't seem to map to tables I have tried to search through?

Redirecting output to a file makes it readable, other tools cli help don't have same issue, and I have tried different terminals and fonts. Not sure what could be causing this, and it may just be on my end.

Steps to Reproduce the Problem

  1. Install JJ
  2. jj -h
  3. view output

Expected Behavior

The help looks normal, as it does when redirected to a file and opened

Actual Behavior

Will display correctly if I redirect output to a file. I have tried several fonts including system defaults.

The output in terminal looks like this:

[[1m[[4mUsage:[[0m [[1mjj[[0m [OPTIONS] <COMMAND>

[[1m[[4mCommands:[[0m
  [[1mabandon[[0m      Abandon a revision
  [[1mbackout[[0m      Apply the reverse of a revision on top of another revision
  [[1mbookmark[[0m     Manage bookmarks
  [[1mcommit[[0m       Update the description and create a new change on top [aliases:
ci]

Specifications

  • Platform: Alpine Linux
  • Version: jj 0.22.0
@ilyagr
Copy link
Collaborator

ilyagr commented Oct 31, 2024

Perhaps you are using less without -R somehow? Does jj -h |less -R look better? What about jj -h |cat (or, equivalently, jj --no-pager -h, but jj -h --no-pager currently does not work)?

Separately, it seems that jj -h --color=none does not disable the ANSI escapes in the help. This is a minor problem I discovered while debugging this; perhaps clap has some way to fix it. Update: I posted this as #4746.

Update: You can also use jj --color=none -h, which does work in spite of #4746, though only in jj 0.22+

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

Yes,jj -h |less -R does work as expected. jj -h --no-pager did not work, and --color=none did not work in my version (0.22.0)

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

It might be --color=never.

You should look at whether you have jj's ui.pager set, and the environment variable PAGER, as well as LESS.

One config that should work is to set PAGER=less and LESS=FRX. ui.pager can then be unset or also set to less.

If you also want mouse scrolling (which has some downsides) and a different status bar, my actual config is LESS="R FX M --mouse --wheel-lines=2"

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

I'd also appreciate knowing what these things are currently set to for you, and why (is that your config or Alpine's?). I'm wondering if we can add something about this in the FAQ. It often happens on Windows.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I just installed (clean) from APK (Alpine Package Keeper), and typed jj -h to view cli docs as I do for every other tool without configuration or issue. Been planning to test with forgejo forever. Finally getting to it.

I think if one needs to do configuration for JJ cli help, it may be doing too much on the formatting end haha.

If I want to scroll through long docs, I will do it in a browser, though all for local docs, just think that the default cli help should not require formatting on any platform. I also use many tools built with clap, and this is first time I am seeing anything like this.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Yeah, I might try it with an Alpine VM one day (never used it, but hopefully it's not too hard to run). I'm currently not sure what jj could do differently. We've been discussing setting LESS=FRX ourselves in some cases, but it's not clear how to do it so that it always solves more problems than it causes.

In any case, I appreciate the report! Good to know that something weird is going on with jj and Alpine.

@ilyagr ilyagr changed the title CLI --help font or character format trouble Alpine Linux: CLI --help font or character format trouble Nov 1, 2024
@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Can you post the output of printf "%s\n" "$PAGER" "$LESS" $(jj config get ui.pager 2>&1)?

Update: Edited, had a bug before.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I frankly don't understand why Less is being used at all for cli help. Why would you be displaying the contents of a file instead of building the help out through clap?

This is the fist time I am seeing anything like this, and it is frankly very annoying on the UX aspect.

The output states:

less

less

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I would expect to be able to transition through cli help at every step of the command, not just read some file:

e.g. jj -h, jj somecommand -h, jj somecommand somesubcommand -h

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

ignore last statement, seems I can get that level of help still, but it is just feels weird relative to all other cli help. The needing to exit instead of just remaining on cli is odd to me.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Thanks! Setting LESS=FRX should fix your problem. You can preview the effect by running LESS=FRX jj help ....

We are building the help with clap. It gets long sometimes, e.g. jj help rebase, so we page it. (Aside: technically, there is no file involved, just pipes) If you use LESS=FRX (as opposed to just LESS=R), it will act as though there was no pager whenever the output is short.

As you observed, this is not a perfect approach in all cases. Again, it's a lot better with LESS=FRX. Barring that, I recommend the trick of jj help rebase |cat to suppress paging in jj or most programs that use this approach.

It turns out to not be easy to make this work well for 100% of configs out of the box, but we'll do better if we can.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

I think you can also set ui.pager to less -FRX to fix this just for jj.

#836 is related. I am not sure why it's not working for you, actually.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

But the need to page is the off design choice for cli-help to me. You could always link to longer format more comfortable local or remote docs for long form info. I come to cli for quick information, and to remember commands. Is the paging necessary because you want to show all commands and options through cli help?

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Do you configure ui.pager in your config.toml? If yes, does removing that config help?

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

But the need to page is the off design choice for cli-help to me. You could always link to longer format more comfortable local or remote docs for long form info. I come to cli for quick information, and to remember commands. Is the paging necessary because you want to show all commands and options through cli help?

Clap's convention is to have jj rebase -h be the "short help" and jj help rebase be the "long help". I find it a bit weird, but it is what it is. The long help definitely gets long sometimes.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I don't even know what ui.pager is yet, that is the whole point. Or where the config.toml is for this, assuming ~/.config/jj.

If help is supposed to be the first command that people may run, it shouldn't have any risk of requiring configuration to view it cleanly.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

But the need to page is the off design choice for cli-help to me. You could always link to longer format more comfortable local or remote docs for long form info. I come to cli for quick information, and to remember commands. Is the paging necessary because you want to show all commands and options through cli help?

Clap's convention is to have jj rebase -h be the "short help" and jj help rebase be the "long help". I find it a bit weird, but it is what it is. The long help definitely gets long sometimes.

I am doing -h right now, and having this issue. I have used dozens of tools made with clap, and this is the first time I experience this.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

I am very confused why your ui.pager is set to "less" as opposed to the setup in

https://github.com/martinvonz/jj/blob/91f869c32a7f2fc374c79205a2d43ff1ca1c991c/cli/src/config/misc.toml#L23

This is worth investigating, any help Alpine people can provide is welcome. Or I might run a VM one day.

It's an interesting FR to have jj -h (as opposed to jj help or jj rebase --help) not have paging on by default. I think it has downsides as well as upsides, but I'll mention it explicitly.

You are correct that paging by default is not a clap convention. A few programs for which the help gets long do it; Sapling is the one that comes to mind. I would guess that plenty of people appreciate it, but it is frustrating when it doesn't work right. Sorry you are experiencing all the downsides and none of the upsides.

The config would be at jj config path --user, if it exists, which should be ~/.config/jj/config.toml. You can also look at jj config path --repo.

@emilazy
Copy link
Collaborator

emilazy commented Nov 1, 2024

FWIW, git rebase --help also opens a pager (because it shows you the very long manual page).

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I am not an Alpine person (but it is my chosen distro right now haha), but I do appreciate the discussion and help still, and will do my best to give feedback. Update: Config was empty in XDG_CONFIG_HOME, but setting ui.pager = "LESS" in toml did the trick as you mentioned.

Is the config.toml supposed to be empty on clean install?

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

FWIW, git rebase --help also opens a pager (because it shows you the very long manual page).

Yes, but I don't have this issue with git rebase --help

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Is the config.toml supposed to be empty on clean install?

Yes.

but setting ui.pager = "LESS" in toml did the trick as you mentioned.

You mean ui.pager = "less -FRX"? ui.pager = "LESS" shouldn't work, unless you have a case-insensitive system. (It works for me, but I'm on a Mac).

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

I mean ui.pager = "LESS" haha. And there is nothing to be sorry about, this is exciting. I didn't mean to sound frustrated, just trying to give some feedback in case it my help improve UX.

The capital "LESS" mattered, lowercase did not work.

"less -FRX" also works

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

OK, I started a VM and I can reproduce this.

Does realpath $(which less) say "busybox"? I don't think this is actually a problem, but it was curious. (You can install actual less if you want).

Does setting ui.pager = "LESS" result in an error message on the first line of the output? On second thought, it makes sense why it disabled paging.

I still don't understand why there is no effect from https://github.com/martinvonz/jj/blob/91f869c32a7f2fc374c79205a2d43ff1ca1c991c/cli/src/config/misc.toml#L23 .

I get:

$  jj config list ui |cat
ui.pager="less"

Update: Maybe it's just because Alpine sets PAGER=less. Maybe we should special-case that and not let it override https://github.com/martinvonz/jj/blob/91f869c32a7f2fc374c79205a2d43ff1ca1c991c/cli/src/config/misc.toml#L23 is LESS is not set.

Update 2: Yes, this is the problem. PAGER= jj resolve -h works. So, one way of looking at it is that Alpine sets PAGER=less by default without setting a sane value for LESS. But we could probably detect it and work around it.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

Yes, by default most tools and utils on Alpine are busybox, and it uses openrc by default which is why I decided to give it a go. I have not used or configured less much so I am ignorant in what sane values would be or even this tool honestly. I can create an issue there also, I am in process of setting up environments for daily driving with Alpine, and this would be good to know there if it will affect others.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

I think I have a fix in mind. We can ignore PAGER=less if the environment variable LESS is not set and use our default instead.

This might only solve 95% of the problem for you unless you apk add less, because busybox less doesn't seem to support less's -X option to not clear the screen on short outputs. But it supports FR, which are much more important.

Update: Not sure whether we want to detect the case of PAGER=/usr/bin/less or, worse, PAGER="/c/Program Files/Git/bin/less" (which I'm not sure even would work). This would also call problems if somebody put a script called less in their ~/.local/bin that calls /usr/bin/less -FRX --more_options. As always, nothing is risk-free.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

Problem was solved for me the moment you told me about ui.pager option for config.toml, and now I understand LESS, MORE hehe. Mainly created this issue to deliver ux feedback in case it was valid. Thanks so much @ilyagr

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 1, 2024

Another possibility would be to file an FR/merge request for https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/main/alpine-baselayout/profile to add export LESS=R to (or remove PAGER from) their default profile.

export LESS=FR or FRX could also be considered, but they have some minor downsides that could annoy people (and X doesn't do anything for busybox less), while I can't imagine anybody inconvenienced by export LESS=R.

Update: Actually, I can. If we implemented a workaround as in #4745 (comment), setting LESS to R would break it.

@andar1an
Copy link
Author

andar1an commented Nov 1, 2024

Another possibility would be to file an FR/merge request for https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/main/alpine-baselayout/profile to add export LESS=R to (or remove PAGER from) their default profile.

export LESS=FR or FRX could also be considered, but they have some minor downsides that could annoy people (and X doesn't do anything for busybox less), while I can't imagine anybody inconvenienced by export LESS=R.

Update: Actually, I can. If we implemented a workaround as in #4745 (comment), setting LESS to R would break it.

I don't think the best solution is individual PRs to distros yet, in case Gentoo and other busybox based distros also do similar. May be worth checking Gentoo quickly to see what is up there.

https://github.com/gentoo/baselayout/blob/9f217ba38e0cd0e91e69de5f1f36a0cca682cc4b/etc/profile#L16

@andar1an andar1an changed the title Alpine Linux: CLI --help font or character format trouble Busybox Less: CLI --help font or character format trouble Nov 1, 2024
@ilyagr
Copy link
Collaborator

ilyagr commented Nov 3, 2024

I'll change the title again, since Busybox Less is not a problem here, it's the default /etc/profile on these distributions that jj doesn't know how to handle (setting PAGER=less and no LESS).

#3657 is also closely related.

@ilyagr ilyagr changed the title Busybox Less: CLI --help font or character format trouble CLI --help font or character format trouble when PAGER=less and LESS is unset (Alpine/Gentoo default) Nov 3, 2024
@andar1an
Copy link
Author

andar1an commented Nov 3, 2024

I don't know of it is only related to Gentoo or Alpine defaults, I haven't looked at every distribution that uses Busybox's Less, and viewed their defaul profile.

The issue should still be related to busybox less, because you are making assumptions of how paging is handled, and it requires non-default less options which no other tool I have used on busybox or gnu less requires.

This is the first tool I have ever experienced this with. It leads me to believe JJ is the outlier, not Alpine or Gentoo defaults for profile.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 4, 2024

By the way, thank you for checking on Gentoo's config! It's helpful to know that Alpine is not the only example. You're probably right that changing their config is not the answer then.

I'm not saying this is only the default in Gentoo and Alpine. "More than one distribution" is probably enough info for me.

@purepani
Copy link

purepani commented Nov 8, 2024

I'm also getting this issue in NixOS

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 8, 2024

I'm also getting this issue in NixOS

@purepani , just to double-check, can you run printf "%s\n" "$PAGER" "$LESS" $(jj config get ui.pager 2>&1)?

@purepani
Copy link

purepani commented Nov 8, 2024

[~/nixos-config]$ printf "%s\n" "$PAGER" "$LESS" $(jj config get ui.pager 2>&1)
less

less

@purepani
Copy link

purepani commented Nov 8, 2024

It is also fixed by the env var solutions above(setting LESS=FRX and PAGER=)
Also this isn't mentioned in the issue as far as i can tell, but htis is pretty much every command, not just the help command. It was probably clear, but just wanted to mention it anyway.

@ilyagr
Copy link
Collaborator

ilyagr commented Nov 8, 2024

setting LESS=FRX and PAGER=

You don't need both, either of them should be sufficient by itself. I recommend LESS=FRX and PAGER=less; that should work well for most programs.

Detail: For a few programs (e.g. ranger), LESS=RX is better; for those you can also call less +F instead of F if you have LESS=FRX.

@purepani
Copy link

purepani commented Nov 8, 2024

Yeah I meant both of those individually not together sorry about that haha.

@andar1an andar1an changed the title CLI --help font or character format trouble when PAGER=less and LESS is unset (Alpine/Gentoo default) CLI --help font or character format trouble when PAGER=less and LESS is unset (Alpine/Gentoo/NixOS default) Nov 8, 2024
@gregwebs
Copy link

As a reference point for someone new to jj: I have never had this issue with git so jj seemed broken in comparison. Actually I haven't seen this issue with any other tool I have used and auto-paging is becoming more popular now (although still relatively rare).

In the short term one thing that would help is editing this issue so that the solution (export LESS=FRX worked for me) shows up right at the top rather than having users scroll through this thread.

Git must be doing some kind of detection for this issue?

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

5 participants