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

gh doesn't respect TERM, dumps colors / codes everywhere, breaks in dumb terms when prompting #5721

Open
zenspider opened this issue May 26, 2022 · 20 comments
Labels
accessibility bug Something isn't working help wanted Contributions welcome p3 Affects a small number of users or is largely cosmetic

Comments

@zenspider
Copy link

Describe the bug

9979 % gh --version
gh version 2.11.3 (2022-05-25)
https://github.com/cli/cli/releases/tag/v2.11.3
9980 % gh pr create 
�7�8? Where should we push the 'zenspider/fix/get-remote-branch' branch?  [Use arrows to move, type to filter]
> zenspider/browse-at-remote
  Skip pushing the branch
  Cancel
�7  C-c C-c�8

Steps to reproduce the behavior

  1. open a shell "somewhere dumb". This can be a remote host that doesn't have your shiny mac terminal app, or in can be a shell in emacs, or whatever.
  2. use any subcommand that prompts w/ those damn menus.
  3. See above

Expected vs actual behavior

If the termcap doesn't say you have the abilities to do cursor movements, don't do prompts... probably treat it like there's no TTY on stdin:

9981 % gh pr create < /dev/null
must provide `--title` and `--body` (or `--fill`) when not running interactively

Usage:  gh pr create [flags]
...

Aside: I've found that EVERY damn command line tool written in go is guilty of this behavior and when I've gone to the libraries responsible for these interactions, they've been less than helpful. So, it's really on you to step up and make this tool useable in more than one context. PLEASE don't pass the buck to the go library you're using because they don't give a shit.

Logs

see above

@zenspider zenspider added the bug Something isn't working label May 26, 2022
@cliAutomation cliAutomation added the needs-triage needs to be reviewed label May 26, 2022
@zenspider
Copy link
Author

REALLY what I want is for gh to act more like hub... or hub to continue to tag releases... I can see @mislav and @halostatue fixed the protocol issue and that's would get me back to a working toolset.

@rpdelaney
Copy link

NO_COLOR might be useful here.

@mislav
Copy link
Contributor

mislav commented May 29, 2022

Agreed that TERM=dumb should act as if NO_COLOR=1 was set plus as if there is no interactive TTY. This was also suggested in #5407 but I forgot that there is no open issue for it, so thanks for making one!

@mislav mislav added p3 Affects a small number of users or is largely cosmetic help wanted Contributions welcome and removed needs-triage needs to be reviewed labels May 29, 2022
@zenspider
Copy link
Author

@mislav alternatively or as a workaround... could you cut a new version of hub that has the latest fixes to the git default protocol?

@mislav
Copy link
Contributor

mislav commented Jun 6, 2022

@zenspider Yes, I've been planning to do that too 👍

@zenspider
Copy link
Author

❤️

@robnagler
Copy link

Just got this, which is not respecting $NO_COLOR (let alone $TERM):

^[[0;33mA new release of gh is available:^[[0m ^[[0;36m2.17.0^[[0m → ^[[0;36m2.18.0^[[0m
To upgrade, run: brew upgrade gh
^[[0;33mhttps://github.com/cli/cli/releases/tag/v2.18.0^[[0m

@pepc
Copy link

pepc commented Dec 13, 2023

Just wanted to mention that the issue of not respecting $NO_COLOR or $TERM is not cosmetic when using gh from python to get a JSON file with the --json switch. The json output from gh cannot be parsed with json.loads.

@zenspider
Copy link
Author

9986 $ NOCOLOR=1 TERM=dumb gh repo fork --default-branch-only
! zenspider/git.ruby-lang.org already exists
? Would you like to add a remote for the fork? (y/N) �7

AGAIN: this tool is UNUSABLE when it insists on prompting w/o respecting TERM

@andyfeller this isn't (just) accessibility... this is flat out broken. And it isn't bypassable... not how there's a --remote flag but it doesn't have/respect --no-remote. PLEASE fix this tool. PLEASE fix hub as well. Right now one is broken and the other is getting further and further out of date (I still use hub to cut PRs because it actually works w/ my editor, no prompting, unlike gh).

@zenspider
Copy link
Author

@zenspider Yes, I've been planning to do that too 👍

PLEASE?

@andyfeller
Copy link
Contributor

@zenspider : I understand this issue has been particularly frustrating for you as I'm sure you can understand that the CLI team has been operating under extremely limited constraints.

That said, let's figure out what exactly has to change from a code perspective.

How should TERM affect prompting?

9986 $ NOCOLOR=1 TERM=dumb gh repo fork --default-branch-only
! zenspider/git.ruby-lang.org already exists
? Would you like to add a remote for the fork? (y/N) �7

AGAIN: this tool is UNUSABLE when it insists on prompting w/o respecting TERM

Prompting has been part of the core product design if and when users are missing information, finding that it is a better user experience than just failing when you could gracefully recover.

Various questions:

  1. What about prompting is a problem for various terminals?
  2. How are you expecting TERM to affect the use of prompts?
  3. What alternatives should happen when gh commands are invoked without relevant information in these scenarios?

@robnagler
Copy link

@andyfeller for us, gh cli is often being called from a program so there's no user to prompt, or there might be a higher level command driving the gh cli. It's convenient to use gh cli from Bash program vs writing a specific program using the API, or worse, using curl. Perhaps you could add a flag or environment variable to indicate batch mode, viz. sqlite3 --batch.

@zenspider
Copy link
Author

prompting has been part of the core product design

that just means it has been broken from the start. Literally. I've never successfully made a PR via gh pr create.

I'm uninterested in gh's "core product design"... I'm interested in its functionality or lack thereof. I want it to work as well as hub pull-request, which it seriously can't compete with right now even tho hub has been EOL for quite some time.

All of your questions can be answered by running hub pull-request & friends.

@andyfeller
Copy link
Contributor

@zenspider : Before we carry this conversation further, I'd like to ask you to pause and revisit our code of conduct because I don't think this is the necessary and constructive discussion to address this concern.

If there is someone else you're confident is able to work with us to understand your product needs without having to infer what you mean, I'd be happy to talk with them instead. Otherwise, I would appreciate a modicum of trust that I'm trying to genuinely understand while balancing a full workload.

@andyfeller
Copy link
Contributor

@andyfeller for us, gh cli is often being called from a program so there's no user to prompt, or there might be a higher level command driving the gh cli. It's convenient to use gh cli from Bash program vs writing a specific program using the API, or worse, using curl. Perhaps you could add a flag or environment variable to indicate batch mode, viz. sqlite3 --batch.

@robnagler : each GitHub CLI command should be designed to only prompt if flags aren't used, but that is a soft "should"; having specific use cases that have appeared would be more useful to dive deeper on.

@halostatue
Copy link

@andyfeller I believe that there are fairly explicit requests in this thread for desired behaviour, and I personally do not see anything in what @zenspider said that warranted a warning about the code of conduct.

For him, because the GitHub CLI assumes that the terminal is a "smart" terminal and uses ANSI escape codes even if the value of $TERM indicates a "dumb" terminal that knows nothing about ANSI escape codes. At a minimum, if $TERM / termcap does not indicate an awareness of ANSI escape codes, a command that could prompt should treat stdin as non-interactive. This would potentially show up (as originally reported) in emacs terminal windows, vim/neovim terminal windows, possibly some tmux/screen configurations, etc.

Going beyond this, I personally think it would be useful to have something similar to AWS CLI's --cli-input-json / --cli-input-yaml and --generate-cli-skeleton arguments for most commands accepting interactive input. In this way, one could generate a blank template file, edit it, and read it back into the command the same as if one had specified everything on the command-line as flags or had gone through the menu.

I believe that what @robnagler wants is similar but slightly different. This would be an explicit --non-interactive (--batch) flag which forces any command that could prompt to pretend that stdin is not a TTY.


As a separate but related issue, I believe that there would be value in a systematic comparison between hub and gh to identify missing features — there has been some level of useful feature loss. My main missing feature — which I haven’t bothered reporting anywhere — is the inability to simply do gh repo add remote owner/repo the way you could do hub add remote owner/repo.

@robnagler
Copy link

My request for --batch is a compromise with @andyfeller's position about the philosophical underpinnings of gh cli. @halostatue I greatly prefer your approach of cli's not prompting, except for explicit commands such as generating skeletons or, for example, rclone's configure.

each GitHub CLI command should be designed to only prompt if flags aren't used, but that is a soft "should"; having specific use cases that have appeared would be more useful to dive deeper on.

@andyfeller I think there's some missing history here. I don't want to talk like a graybeard, but I am... There's no TL;DR, sorry.

By repeating some history (below), I hope to give some context to why you may be sensing some agitation on the part of @zenspider and possibly @halostatue. This is important for myriad reasons. I focus on one: programmatic control. I believe folks who agitate for respecting $TERM are trying to keep their logs easily parseable and their development environments effective. There is some conflation of tty and TERM (also below).

History as I see it: we are repeating much of what people learned in the 1970s and 1980s about HCI and separating business from presentation logic. Greatly simplified: There was the DEC culture of highly interactive command lines of Tenex>TOPS20>VMS>Norton Commander (later Windows NT), which was beaten by conceptually simpler, composable Unix (later MacOS). One of the great(est?) inventions in Unix was that a file could be anything, including a pipe. This allowed composability on the command line that was not possible before. IOW, the business logic was in the composable commands (ps, kill, sccs, etc.), and the presentation logic was handled by composition (awk, sed, cut, colrm, sort, etc). This worked and still works.

None of these commands included terminal output. (tty's are separate and discussed below.) Why? There were too many terminals and protocols (so many there's terminals-wiki.org. That's why Bill Joy invented termcap which was a first of its kind library to abstract presentation logic for terminals and allowed programs on Unix to talk to any terminal. Before termcap, you wrote IBM 3270, Tandem 6530, etc. terminals. Programs were written for specific terminals, and they mixed business and presentation logic willy nilly. It was extremely annoying, and I think a big reason why we are "all" using Unix.

@halostatue wrote:

At a minimum, if $TERM / termcap does not indicate an awareness of ANSI escape codes, a command that could prompt should treat stdin as non-interactive.

Actually, that's not true. I first interacted with a computer on a teletype in 1970, which was also when Unix was invented. That's where "tty" comes from, and why there's test -t in Unix and isatty in C. A teletype (and DECwriter more commonly) obviously can't have escape codes, and programs in Unix would print # when you typed a backspace so you'd know that the previous character(s) were deleted. As I noted above, no Unix command line programs assumed they could control the terminal in the 1970s, because there was no standard for terminal control, and for the most part terminals were dumb.

There was still a need to distinguish between a teletype where you could prompt the user and not, and that hand nothing to do with the terminal being dumb. Rather, it had to do with the input being a pipe or a (disk) file. Prompts only work when the the communication is bidirectional through a tty device (file). Back in the day DEC invented pseudo terminals for automation, and it's a powerful tool that many of the people on this thread are using for automation. Just because stdin is a tty, doesn't mean that $TERM is smart. That's what this issue is about: Emacs command shells (in my case), which uses pty's to commands so they know they can prompt and a TERM=dumb so they know they are not talking to a smart terminal, and shouldn't send terminal control codes.

So it's actually a problem to conflate isatty and TERM=dumb, because it reduces some command line efficacy inside Emacs command shells, but that's a finer point to the much larger issues of separating presentation and business logic and composability in shells.

At some point in the 2000's, people "discovered" escape codes and put them in their programs. (I blame it on the (shudders) <blink> tag.) Through the 1990's, there was no problem with escape codes. People just used libraries like curses, tputs, etc. This switched in the 2000's to hardcoding escape codes. In the 2010's this trend accelerated, and Rob Pike's (a little copying)[https://news.ycombinator.com/item?id=14903059) is to blame for proliferation of escape codes in Go. Properly understood, it's a great proverb, but it's quite subtle.

All this has led to NO_COLOR, TERM=dumb, etc. checks like this, and shell environment files like this, which is all caused by people not knowing the reasons for isatty and $TERM, the use of libraries to communicate with terminals, the need for composability in the shell, and letting presentation programs like Emacs, Vim, VSCode, etc. deal with colorization.

So this is why many of us are agitated and why there are myriad complaints in practically every repo for shell commands.

If you've read to this point, thank you.

@williammartin
Copy link
Member

Hi folks.

I want to cover a few things and try to keep this on track. Understandably, there is frustration from @zenspider about the reported behaviour and that it hasn't been addressed in a long time when it's so irritating. It's also understandable that @andyfeller is frustrated when he's trying to bring curiosity to move this stale issue forward on as an extra to what he already had on his plate.

What I'd ask is that we move past some of the higher tension comments above with the understanding that it's in everyone's best interests that we collectively work to resolve this. More pragmatically, as maintainers we have an extensive backlog of both community and GitHub internal work vying for our time so helping us is the best way to get this moving. I mean that both in terms of the content and the tone, otherwise everyone loses. I deeply appreciate the comments that provide us with extra context and direction (see point 2 below).

Addressing some specific points that might help get us all on the same page:

  1. @mislav no longer works at GitHub and the current maintainers nor GitHub have any official affiliation with hub.
  2. In fact, no one is still on the team from the time when decisions under discussion were made or this issue was created. That's not your problem as users of GitHub software but hopefully helps set expectations around us getting up to speed and leaning on the community to help us get there.
  3. There are a few different feature requests and issues recorded in here. On Monday when I return from the weekend I will create separate issues to ensure these get tracked, and hopefully we can keep this focused on the making sure we provide better dumb terminal support. It's gonna get wild in here otherwise.
  4. Just because there's hopefully an answer already, regarding --non-interactive, I believe this is the purpose of the GH_PROMPT_DISABLED env var (can be discovered in gh environment) but this was introduced 4 years ago (long before our time) and it’s possible there are places this doesn’t work (and if so we'd like to hear about them).

I hope that provides some clarity to where we are and helps this thing get resolved. Cheers all.

@halostatue
Copy link

  1. There are a few different feature requests and issues recorded in here. On Monday when I return from the weekend I will create separate issues to ensure these get tracked, and hopefully we can keep this focused on the making sure we provide better dumb terminal support. It's gonna get wild in here otherwise.

I think that there’s really only the one: if the terminal does not explicitly support the rich interactive codes‡ used by gh for its prompting , then unless and until gh supports "dumb" prompting without the terminal controls, prompting should be disabled. Ideally "dumb" prompting would be possible (essentially repeated printf and gets calls) for at least some built-in commands. It sounds like, based on #4, if $TERM is dumb, then GH_PROMPT_DISABLED should probably be treated as on until this is the case.

‡ Trying to better distinguish the distinction, @robnagler. The comment you quoted was simply a shorthand for what I think would work well for gh.

I mentioned other gh / hub disparities and an explicit comparison primarily because I believe it would be useful to find some of the things that have been misplaced between gh and hub. gh is a great tool, but it could and should work better in constrained environments.

(I believe that I could also create a gh-remote extension that more or less wraps git remote but understands GH shorthand the way that hub remote does, so it’s not that urgent.)

@darkfeline
Copy link

For the people who aren't knowledgeable about TERM, TERM=dumb basically means you don't have a terminal. You can read text from stdin, N characters at a time. You can write text to stdout (and stderr), N characters at a time. That's it.

No arrow key support. No ANSI escape codes (which are just garbage that gets written out as-is). Think typewriter hooked up to a modem.

TERM=dumb is useful in a variety of situations, such as in scripting and automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility bug Something isn't working help wanted Contributions welcome p3 Affects a small number of users or is largely cosmetic
Projects
None yet
Development

No branches or pull requests

10 participants