slang: respect TERMINFO_DIRS and ignore FHS paths #50976
Merged
+174
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Success on aarch64-linux (full log) Attempted: slang Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: slang Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: slang Partial log (click to expand)
|
lheckemann
force-pushed
the
slang-terminfo
branch
from
5416342 to
688d769
Compare
|
Rebased on staging due to linux mass-rebuild. |
|
Success on x86_64-linux (full log) Attempted: slang Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: slang Partial log (click to expand)
|
symphorien
reviewed
Nov 24, 2018
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: slang Partial log (click to expand)
|
|
I wonder if anyone needs those FHS paths? On NixOS they don't exist, but on some Nix systems they could be useful. I suppose we can just have everyone that needs it set them in TERMINFO_DIRS. |
|
It's possible that someone might need them. If they do, I suggest adding them to TERMINFO_DIRS in their env, which gets them searched. Not removing them from the source makes it impossible to achieve the purer behaviour, while removing them doesn't completely prevent host system terminfo from being detected, so I'm not in favour of adding them back. |
lheckemann
force-pushed
the
slang-terminfo
branch
from
688d769 to
1e5c61b
Compare
|
Documented this rationale in the commit message. |
|
Success on x86_64-linux (full log) Attempted: slang Partial log (click to expand)
|
|
Failure on x86_64-darwin (full log) Attempted: slang Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: slang Partial log (click to expand)
|
|
Darwin failure seems to be unrelated to my changes, looks like resource exhaustion or something on the build machine. |
|
Can you try to get the |
|
I can, but don't have the time currently. I'm fairly sure upstream won't accept it as is since it's quite a break from their coding standards (whatever they may be) and likely disregards some compatibility issues which we don't care about because we don't run on obscure unix platforms. |
|
well |
|
I have sent it to the author. |
|
I meant compatibility as in portability of the C code, not appropriateness of the behaviour. Thanks though! |
|
I got a response today.
|
|
Oh nice, thanks! |
|
Maybe one day a version with the fix will be released 🤣 |
|
We could also just pull their patch in this case. |
lheckemann
requested review from
basvandijk,
Ericson2314,
FRidh,
infinisil,
matthewbauer,
thoughtpolice and
ttuegel
as code owners
lheckemann
removed request for
thoughtpolice,
matthewbauer,
ttuegel,
basvandijk,
Ericson2314,
FRidh and
infinisil
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Makes whiptail, nmtui and others work correctly in nonstandard terminals like alacritty.
cc @tilpner who was complaining about this not working, @Fuuzetsu maintainer
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)