guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#53257] [PATCH] gnu: foot: Wrap the program to expose TERMINFO_DIRS


From: Kevin Boulain
Subject: [bug#53257] [PATCH] gnu: foot: Wrap the program to expose TERMINFO_DIRS
Date: Mon, 28 Feb 2022 21:29:42 +0100

Thank you, I'll send a follow-up patch to resolve the concerns (that's
greatly appreciated, I'm new at both Guix and Scheme).
I did test it with an ncurses program so I can confirm it works (no
complaints about an unknown terminal anymore).

I only have a single concern about this approach: native-search-paths
made the terminfo database somewhat 'global' (as long the shell
sourced the Guix profile) and it made stuff like SSH less awkward: by
installing foot on both the client and the server, ncurses
applications could 'just work'. Now, by making the terminfo database
only visible inside the terminal itself, SSH is pretty much never
guaranteed to work seamlessly (however, foot appears to be compatible
with xterm so it's not that much of an issue:
https://codeberg.org/dnkl/foot/src/branch/master/INSTALL.md#terminfo).
I agree this has always been a pain point with ncurses programs and
the proper solution would be to upstream the terminal's description to
terminfo so it's easily available everywhere (or people can always
install it manually), but I prefer following the recommendations of
the maintainers here :)

On Sun, 27 Feb 2022 at 20:22, Maxime Devos <maximedevos@telenet.be> wrote:
>
> Kevin Boulain schreef op zo 27-02-2022 om 19:34 [+0100]:
> > +       (modify-phases %standard-phases
> > +         (add-after 'install 'wrap-program
> > +           (lambda* (#:key inputs outputs #:allow-other-keys)
> > +             (let* ((out (assoc-ref outputs "out")))
> > +               ;; footclient executes programs under the server process,
> > +               ;; there is no need to wrap it too.
> > +               (wrap-program (string-append out "/bin/foot")
> > +                             `("TERMINFO_DIRS" ":" prefix
> > +                               (,(string-append out 
> > "/share/terminfo"))))))))))
>
>
> You'll need to add 'bash-minimal' to 'inputs', such that this works
> even when cross-compiling.  I think "./pre-inst-env guix lint foot"
> would warn about this.
>
> Also, this can be simplified a bit, to
>
>      ,#(modify-phases %standard-phases
>          (add-after 'install 'wrap-program
>            (lambda _
>              (let ((out #$output))
>                ;; footclient executes programs under the server process,
>                ;; there is no need to wrap it too.
>                (wrap-program (string-append out "/bin/foot")
>                              `("TERMINFO_DIRS" ":" prefix
>                                (,(string-append out 
> "/share/terminfo"))))))))))
>
> or, reducing the whitespace:
>
>
>      ,#(modify-phases %standard-phases
>          (add-after 'install 'wrap-program
>            (lambda _
>              (define out #$output)
>              ;; footclient executes programs under the server process,
>              ;; there is no need to wrap it too.
>              (wrap-program (string-append out "/bin/foot")
>                            `("TERMINFO_DIRS" ":" prefix
>                              (,(string-append out "/share/terminfo")))))))))
>
> (you'll need to add (guix gexp) to the list of imports to do this)
>
> YMMV on whether this is an improvement.
>
> I hope you don't mind, I went ahead and used wrap-program as
> discussed (I was encountering this issue and had a very similar patch
> as the OP's). Did I get the idea that was discussed in this thread
> right?
>
> Yes, this was the idea, though to be 100% sure it would need to be tested (by 
> running "guix shell --pure foot -- foot" and then running
> ~/.guix-profile/bin/SOME-NCURSES-APP in the terminal, or something like that.
>
>  If yes, should I send another patch to fix the other terminals
> (e.g.:
>
> https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/terminals.scm?id=85a5110de79f4fe9fd822ede3915654ee699d6c5#n220
> )?
>
> That would be nice, but keep in mind that this might not be needed
> for every terminal app -- some apps set TERMINFODIR automatically (I forgot 
> which) and hence don't need any wrapping.
>
> Greetings,
> Maxime.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]