all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Kevin Boulain <kevinboulain@gmail.com>
To: Maxime Devos <maximedevos@telenet.be>
Cc: 53257@debbugs.gnu.org
Subject: [bug#53257] [PATCH] gnu: foot: Wrap the program to expose TERMINFO_DIRS
Date: Mon, 28 Feb 2022 21:29:42 +0100	[thread overview]
Message-ID: <CABaj1X_0LTu1GPJHweWNgge0HCfL=NSqd36c-LyiutGA5xoOJg@mail.gmail.com> (raw)
In-Reply-To: <8372c3d084580f785710cdc8c0ebdcb92f45ee51.camel@telenet.be>

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	other threads:[~2022-02-28 20:31 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-14 14:02 [bug#53257] [PATCH] gnu: foot: expose terminfo dirs via native-search-paths florhizome
2022-01-14 19:02 ` Maxime Devos
2022-01-14 22:52   ` Florian
2022-01-15 11:45     ` Maxime Devos
2022-01-15 11:48     ` Maxime Devos
2022-01-15 12:30     ` Maxime Devos
2022-01-15 15:19       ` Florian
2022-01-15 15:38         ` Maxime Devos
2022-01-15 18:46           ` Florian
2022-01-23 21:26             ` Maxime Devos
2022-01-15 15:46         ` Maxime Devos
2022-01-15 15:46         ` Maxime Devos
2022-01-15 14:24     ` Maxime Devos
2022-01-28 22:34       ` Ludovic Courtès
2022-02-08 12:46         ` Maxime Devos
2022-02-10 20:30           ` Ludovic Courtès
2022-02-10 21:45             ` Maxime Devos
2022-02-12 21:49               ` Ludovic Courtès
2022-02-27 18:34 ` [bug#53257] [PATCH] gnu: foot: Wrap the program to expose TERMINFO_DIRS Kevin Boulain
2022-02-27 18:41   ` Kevin Boulain
2022-02-27 19:22   ` Maxime Devos
2022-02-28 20:29     ` Kevin Boulain [this message]
2022-03-01 19:34     ` Kevin Boulain
2022-03-01 19:28 ` Kevin Boulain
2022-06-19  5:27   ` Tom Fitzhenry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABaj1X_0LTu1GPJHweWNgge0HCfL=NSqd36c-LyiutGA5xoOJg@mail.gmail.com' \
    --to=kevinboulain@gmail.com \
    --cc=53257@debbugs.gnu.org \
    --cc=maximedevos@telenet.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.