Terminfo parser #50797

tecosaur · 2023-08-04T13:17:33Z

This was part of #49586, but it has been spun out so it an be discussed separately and once merged simplify #49586 a bit.

This introduces a pure-Julia terminfo parser, which means that:

We can no longer depend on calling tput
We can start thinking about fancier terminal-adaptive functionality (see AnnotatedStrings, and a string styling stdlib #49586)

I am hoping this can be considered and (hopefully) merged in the near future 🙂.

Oh, and I think it's fun to note that it looks like this may well be the most concise standards-compliant Terminfo parsers in existence.

This is part of the groundwork for styled printing of StyledStrings.

tecosaur · 2023-08-04T13:21:16Z

One extra complication probably worth thinking about: with this, do we want to bundle a terminfo file (likely xterm256-color, based on microsoft/terminal#6045) to be used with the Windows terminal? (but I think this can be sorted out later)

vtjnash

LGTM, with a couple small nits

base/terminfo.jl

base/terminfo_data.jl

tecosaur · 2023-08-08T13:48:58Z

Thanks for the review Jameson 🙂.

vtjnash · 2023-08-08T14:33:56Z

base/terminfo_data.jl

+    TermCapability(:kf61,     :key_f61,                   "F61 function key"),
+    TermCapability(:kf62,     :key_f62,                   "F62 function key"),
+    TermCapability(:kf63,     :key_f63,                   "F63 function key"),
+    TermCapability(:el1,      :clr_bol,                   "Clear to beginning of line"),


lowercase from here to the end, or is this intentional too?

If you're talking about the inconsistent capitalization with the description strings, that's a consequence of the source I copied from.

base/terminfo.jl

tecosaur · 2023-08-15T19:09:10Z

At this point, short of any last-minute major issues, I think this should be good to go :)

tecosaur · 2023-08-18T15:27:07Z

I'm rather confused by the test errors, depending on the platform it's:

  Got exception outside of a @test
  LoadError: IOError: stream is closed or unusable
  Stacktrace:
    [1] check_open
      @ Base .\stream.jl:388 [inlined]

  Got exception outside of a @test
  LoadError: MethodError: Cannot `convert` an object of type
    Int32 to an object of type
    Union{Bool, Int64, String}

  Got exception outside of a @test
  LoadError: IOError: write: i/o error (EIO)
  Stacktrace:
    [1] uv_write(s::Base.TTY, p::Ptr{UInt8}, n::UInt64)
      @ Base ./stream.jl:1066

  Got exception outside of a @test
  LoadError: IOError: read: i/o error (EIO)
  Stacktrace:
    [1] wait_readnb(x::Base.TTY, nb::Int32)
      @ Base ./stream.jl:410

Test Failed at /usr/home/julia/.buildkite-agent/builds/freebsd13-amdci6-1/julialang/julia-master/julia-7925e17ab7/share/julia/stdlib/v1.11/REPL/test/repl.jl:880
  Expression: output == "julia> "
   Evaluated: "ERROR: UndefVarError: `isunix` not defined\r\nStacktrace:\r\n [1] find_terminfo_file(term::String)\r\n   @ Base ./terminfo.jl:218\r\n [2] load_terminfo(term::String)\r\n   @ Base ./terminfo.jl:238\r\n [3] (::Base.var\"#1042#1044\"{Bool, Symbol, Bool})(REPL::Module)\r\n   @ Base ./client.jl:419\r\n [4] #invokelatest#2\r\n   @ Base ./essentials.jl:887 [inlined]\r\n [5] invokelatest\r\n   @ Base ./essentials.jl:884 [inlined]\r\n [6] run_main_repl(interactive::Bool, quiet::Bool, banner::Symbol, history_file::Bool, color_set::Bool)\r\n   @ Base ./client.jl:417\r\n [7] exec_options(opts::Base.JLOptions)\r\n   @ Base ./client.jl:334\r\n [8] _start()\r\n   @ Base ./client.jl:554\r\n" == "julia> "
Test Failed at /usr/home/julia/.buildkite-agent/builds/freebsd13-amdci6-1/julialang/julia-master/julia-7925e17ab7/share/julia/stdlib/v1.11/REPL/test/repl.jl:889
  Expression: output == "1\r\nexit()\r\n1\r\n\r\njulia> "
   Evaluated: "" == "1\r\nexit()\r\n1\r\n\r\njulia> "

So uh, I guess I'll leave this PR alone and hope.

tecosaur · 2023-08-18T15:43:26Z

That said, I see what the isunix is undefined is on about, I forgot a Sys. and that's an easy fix.

tecosaur · 2023-08-18T22:59:13Z

Ok, I think ignoring the spurious faliures there's one genuine one to address. the Windows i686 Mingw-32 entry, which has:

Error During Test at C:\buildkite-agent\builds\win2k22-amdci6-1\julialang\julia-master\julia-192e32e9be\share\julia\test\testdefs.jl:21
  Got exception outside of a @test
  LoadError: MethodError: Cannot `convert` an object of type
    Int32 to an object of type
    Union{Bool, Int64, String}
  Closest candidates are:
    convert(::Type{T}, !Matched::T) where T
     @ Base Base.jl:84
  Stacktrace:
    [1] setindex!(h::Dict{Symbol, Union{Bool, Int64, String}}, v0::Int32, key::Symbol)
      @ Base .\dict.jl:374
    [2] Dict{Symbol, Union{Bool, Int64, String}}(::Pair{Symbol, Bool}, ::Vararg{Pair})
      @ Base .\dict.jl:93
    [3] top-level scope
      @ C:\buildkite-agent\builds\win2k22-amdci6-1\julialang\julia-master\julia-192e32e9be\share\julia\test\terminfo.jl:30
    [4] include
      @ Main .\Base.jl:493 [inlined]

Not sure what's going on / what to do with this though.

test/terminfo.jl

tecosaur · 2023-08-19T14:31:26Z

Ok, now it looks like there are just libgit2 errors??

Implement a terminfo parser, and use it to replace the tput calls. While tput calls are fairly safe, given tput it part of the UNIX specification, it's nice to remove a system dependency and call one less executable. More crucially, switching to parsing terminfo ourselves makes it much more convenient to check terminfo capabilities. A try-catch wrapped success(`tput termcap`) call can now be replaced with a simple haskey(current_terminfo, :termcap) or get(current_terminfo, :termcap, false) as appropriate. This is part of the groundwork for styled printing of StyledStrings.

The terminfo files are taken from my local device, and the assertions are generated from the current terminfo implementation but have been partially manually checked for correctness. As such, this is mainly a barrier against potential implementation drift in the future, as unlikely as it may be.

The idea is still sound, but the REPL stdlib is not available in Base, and REPL.Terminals.raw! is non-trivial. Since this is the final fallback, and it's also been a bit flakey on various terminal emulators, we'll just remove it for now.

tecosaur · 2023-08-19T19:39:13Z

Looks like force pushing with a trivial tweak to re-trigger CI makes the libgit2 issue go away, I'm guessing it was just transient.

tecosaur · 2023-08-19T19:55:28Z

Thanks guys for giving this a look and helping me get the last few niggles sorted :)

Seelengrab · 2023-08-19T20:11:24Z

Oh I'm SO looking forward to better terminal string styling in Base!

caleb-allen · 2023-08-19T20:12:31Z

Thanks for your work on this @tecosaur! Very exciting progress

tecosaur · 2023-08-19T20:16:38Z

Oh, I do see https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#git-recommendations-for-pull-request-reviewers, but it would have been nice if this was not-squashed. I generally put effort into nice discrete commits with descriptive messages — I rebase changes into the appropriate commit for the sake of a nice history. As it stands, I see those commits and the messages I wrote have been entirely lost, and the commit data is clobbered with GitHub's generated data.

I don't think this is a big deal with this PR, but with say the styled strings PR I think it's very much worth keeping the commits I've curated.

Seelengrab · 2023-08-19T20:57:46Z

There is the don't squash label for that.

tecosaur · 2023-08-19T21:45:23Z

That's nice, is there any way for that to automatically be applied to my PRs since I can't add labels myself?

vchuravy · 2023-09-20T20:48:20Z

While looking at #51399 I was wondering if terminfo could be it's own standard library?
We only parse it when we are about to start a REPL, no?

tecosaur · 2023-09-21T01:33:40Z

It depends on how the StyledStrings PR shakes out, because we'll then want terminfo before printing.

tecosaur · 2023-09-21T01:35:39Z

Looking at your #51399, would you like me to remove the broadcasting in #51198?

tecosaur · 2023-09-21T01:36:43Z

Oh, I suppose I can just rebase and deal with the merge conflict if needed.

vchuravy · 2023-09-21T01:43:42Z

because we'll then want terminfo before printing.

Yeah I am more and more convinced that the rendering should be in the Client/REPL.

tecosaur · 2023-09-21T02:19:09Z

What about stacktrace printing?

oscardssmith · 2023-09-21T02:30:30Z

IMO fancy stacktrace printing also belongs in the REPL. in a non-interactive mode, our stacktraces shouldn't have color or abreviate or do anything else fancy.

tecosaur · 2023-09-21T02:45:56Z

Cool, 👍 sounds reasonable to me — just want to make sure we're all on the same page regarding what this means.

It would be good to actually organise a triage-y call about StyledStrings and printing at an hour that's actually sane for me 😅.

KristofferC · 2023-09-21T05:19:46Z

IMO fancy stacktrace printing also belongs in the REPL. in a non-interactive mode, our stacktraces shouldn't have color or abreviate or do anything else fancy.

Why wouldn't you want to have colors for the stacktrace when you are executing a file?

vchuravy · 2023-09-21T14:50:37Z

Why wouldn't you want to have colors for the stacktrace when you are executing a file?

Currently colored printing outside the REPL is opt-in.

E.g. try:

vchuravy@odin ~/b/julia> julia --color=yes -e "@info \"msg\""
[ Info: msg
vchuravy@odin ~/b/julia> julia -e "@info \"msg\""
[ Info: msg

We mostly do that because their are many places where colored output will make the logs hard to read.
My comment about moving terminfo out is that currently we only parse it when we start a REPL.

KristofferC · 2023-09-21T15:19:06Z

Currently colored printing outside the REPL is opt-in.

E.g. try:

Ok, what should I see?

We mostly do that because their are many places where colored output will make the logs hard to read.

That should depend on the IO type, not REPL use vs script use?

vchuravy · 2023-09-21T15:30:06Z

Ah my mistake I had done --color=no by accident. Ignore me.

Seelengrab · 2023-09-21T15:33:18Z

I don't have --color=no, and I see this:

on

julia> versioninfo()
Julia Version 1.11.0-DEV.388
Commit 10599b104e1* (2023-09-01 07:08 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: 24 × AMD Ryzen 9 7900X 12-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-15.0.7 (ORCJIT, znver3)
  Threads: 34 on 24 virtual cores
Environment:
  JULIA_PKG_USE_CLI_GIT = true

I do agree that the later two should be colored by default, if the terminal supports it and there is no --color=no set. Isn't that what this PR is about?

tecosaur · 2023-09-21T15:57:00Z

This PR is about obtaining information about the current terminal by inspecting the relevant terminfo file instead of calling tput. The three main benefits of it are:

No longer needing to worry about differences between linux and bsd tput codes
Removing reliance on an externally managed executable
Making it easier to do more advanced terminal-dependant behaviour in the future

Add method of checking tty for truecolor support

80c5371

This is part of the groundwork for styled printing of StyledStrings.

tecosaur mentioned this pull request Aug 4, 2023

AnnotatedStrings, and a string styling stdlib #49586

Merged

brenhinkeller added the feature Indicates new feature / enhancement requests label Aug 4, 2023

tecosaur mentioned this pull request Aug 7, 2023

Meta-issue for improving the REPL experience #50817

Open

15 tasks

vtjnash approved these changes Aug 8, 2023

View reviewed changes

base/terminfo.jl Outdated Show resolved Hide resolved

base/terminfo.jl Outdated Show resolved Hide resolved

base/terminfo_data.jl Outdated Show resolved Hide resolved

tecosaur force-pushed the terminfo-parser branch from fec3fe1 to 15801b5 Compare August 8, 2023 13:46

vtjnash approved these changes Aug 8, 2023

View reviewed changes

vtjnash reviewed Aug 8, 2023

View reviewed changes

base/terminfo.jl Outdated Show resolved Hide resolved

tecosaur force-pushed the terminfo-parser branch from 15801b5 to aafa946 Compare August 8, 2023 15:32

JeffBezanson reviewed Aug 9, 2023

View reviewed changes

base/terminfo.jl Outdated Show resolved Hide resolved

tecosaur force-pushed the terminfo-parser branch from aafa946 to 7925e17 Compare August 15, 2023 08:21

vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Aug 15, 2023

tecosaur force-pushed the terminfo-parser branch 2 times, most recently from 201b203 to 192e32e Compare August 18, 2023 21:11

oscardssmith reviewed Aug 19, 2023

View reviewed changes

test/terminfo.jl Outdated Show resolved Hide resolved

tecosaur force-pushed the terminfo-parser branch 2 times, most recently from 48db274 to 46ab691 Compare August 19, 2023 10:49

tecosaur added 3 commits August 19, 2023 15:32

Remove DCS-based TTY truecolor test

2205231

The idea is still sound, but the REPL stdlib is not available in Base, and REPL.Terminals.raw! is non-trivial. Since this is the final fallback, and it's also been a bit flakey on various terminal emulators, we'll just remove it for now.

tecosaur force-pushed the terminfo-parser branch from 46ab691 to 2205231 Compare August 19, 2023 14:32

oscardssmith merged commit 2690ca8 into JuliaLang:master Aug 19, 2023

oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Aug 19, 2023

maleadt mentioned this pull request Aug 30, 2023

terminfo parser cannot handle ncurses 6.4's screen.xterm-256color #51110

Closed

tkemmer mentioned this pull request Sep 5, 2023

terminfo parser: extendedterminfo() results in conversion error for TERM=screen-256color #51190

Closed

vchuravy mentioned this pull request Oct 6, 2023

Should TermInfo parsing be a stdlib? #51618

Open

tecosaur deleted the terminfo-parser branch October 21, 2023 13:01

KristofferC mentioned this pull request Oct 26, 2023

$TERM supports 256 colors, but don't have colors KristofferC/OhMyREPL.jl#333

Closed

ararslan mentioned this pull request Jul 27, 2024

terminfo parser is ineffective on FreeBSD #55274

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Terminfo parser #50797

Terminfo parser #50797

tecosaur commented Aug 4, 2023

tecosaur commented Aug 4, 2023 •

edited

Loading

vtjnash left a comment

tecosaur commented Aug 8, 2023

vtjnash Aug 8, 2023

tecosaur Aug 8, 2023

tecosaur commented Aug 15, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 19, 2023 •

edited

Loading

tecosaur commented Aug 19, 2023

tecosaur commented Aug 19, 2023

Seelengrab commented Aug 19, 2023

caleb-allen commented Aug 19, 2023

tecosaur commented Aug 19, 2023

Seelengrab commented Aug 19, 2023

tecosaur commented Aug 19, 2023

vchuravy commented Sep 20, 2023

tecosaur commented Sep 21, 2023

tecosaur commented Sep 21, 2023

tecosaur commented Sep 21, 2023

vchuravy commented Sep 21, 2023

tecosaur commented Sep 21, 2023

oscardssmith commented Sep 21, 2023

tecosaur commented Sep 21, 2023

KristofferC commented Sep 21, 2023

vchuravy commented Sep 21, 2023

KristofferC commented Sep 21, 2023

vchuravy commented Sep 21, 2023

Seelengrab commented Sep 21, 2023 •

edited

Loading

tecosaur commented Sep 21, 2023

Terminfo parser #50797

Terminfo parser #50797

Conversation

tecosaur commented Aug 4, 2023

tecosaur commented Aug 4, 2023 • edited Loading

vtjnash left a comment

Choose a reason for hiding this comment

tecosaur commented Aug 8, 2023

vtjnash Aug 8, 2023

Choose a reason for hiding this comment

tecosaur Aug 8, 2023

Choose a reason for hiding this comment

tecosaur commented Aug 15, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 18, 2023

tecosaur commented Aug 19, 2023 • edited Loading

tecosaur commented Aug 19, 2023

tecosaur commented Aug 19, 2023

Seelengrab commented Aug 19, 2023

caleb-allen commented Aug 19, 2023

tecosaur commented Aug 19, 2023

Seelengrab commented Aug 19, 2023

tecosaur commented Aug 19, 2023

vchuravy commented Sep 20, 2023

tecosaur commented Sep 21, 2023

tecosaur commented Sep 21, 2023

tecosaur commented Sep 21, 2023

vchuravy commented Sep 21, 2023

tecosaur commented Sep 21, 2023

oscardssmith commented Sep 21, 2023

tecosaur commented Sep 21, 2023

KristofferC commented Sep 21, 2023

vchuravy commented Sep 21, 2023

KristofferC commented Sep 21, 2023

vchuravy commented Sep 21, 2023

Seelengrab commented Sep 21, 2023 • edited Loading

tecosaur commented Sep 21, 2023

tecosaur commented Aug 4, 2023 •

edited

Loading

tecosaur commented Aug 19, 2023 •

edited

Loading

Seelengrab commented Sep 21, 2023 •

edited

Loading