mlterm: Add support for many build configuration variables #226218
Conversation
doronbehar
force-pushed
the
pkg/mlterm-wayland
branch
2 times, most recently
from
88b595b
to
498853d
Compare
ofborg
bot
added
11.by: package-maintainer
| nativeBuildInputs = [ | ||
| pkg-config | ||
| autoconf | ||
| ] ++ addIfHas "mlconfig" toolsChoices [ |
There was a problem hiding this comment.
Something tells me this is not the best way to do this.
It looks like searching a hash table.
There was a problem hiding this comment.
I'd prefer individual boolean flags rather than list flags. Lists don't compose very well with overrides.
There was a problem hiding this comment.
I'd prefer individual boolean flags rather than list flags. Lists don't compose very well with overrides.
Composing overrides indeed that would have been ideal, but these lists are not that long, and users using them should anyway read the source code, and choose the list they want to be compiled.
Something tells me this is not the best way to do this. It looks like searching a hash table.
I'm open to suggestions. Unless you'd both insist that it's better to use boolean flags.
There was a problem hiding this comment.
Discussion on a third alternative which you already implemented: #226218 (comment)
There was a problem hiding this comment.
I suggest inline this, using lib.elem x list
There was a problem hiding this comment.
I suggest inline this, using
lib.elem x list
That was I missing, thanks!
There was a problem hiding this comment.
Result of nixpkgs-review pr 226218 run on aarch64-darwin 1
2 packages failed to build:
- mlterm
- mlterm-wayland
For Darwin support, we need to disable Xorg and enable the quartz GUI toolkit.
Despite all my nitpickings and change requests, I am absolutely in support of this PR; this is great!
| libX11 | ||
| gtk | ||
| vte | ||
| ] ++ lib.optionals (imagelibChoice == "gdk-pixbuf") [ |
There was a problem hiding this comment.
Nit: If you use optionals that often, just inherit it from lib once and stop the lib. spam.
There was a problem hiding this comment.
Nit: If you use
optionalsthat often, just inherit it from lib once and stop thelib.spam.
I don't mind this spam that much personally, I copy paste these lines anyway, and it will save me only 4 letters. Will do it if you insist.
There was a problem hiding this comment.
I won't insist, it just goes against the DRY principle.
doronbehar
force-pushed
the
pkg/mlterm-wayland
branch
from
77e2061
to
566f585
Compare
| libX11 | ||
| gtk | ||
| vte | ||
| ] ++ lib.optionals (imagelibChoice == "gdk-pixbuf") [ |
There was a problem hiding this comment.
I won't insist, it just goes against the DRY principle.
|
mlterm now builds on Darwin as well, according to ofborg. |
|
I have a branch with most commits squashed, ready to be pushed here. |
github-actions
bot
added
6.topic: emacs
github-actions
bot
removed
6.topic: python
6.topic: rust
6.topic: haskell
6.topic: TeX
Which one? I pushed the rebased branch, with fewer commit messages. |
Remove deprecated `Application` category and make `comment` different then `name`.
doronbehar
force-pushed
the
pkg/mlterm-wayland
branch
from
c555c0b
to
b633ffb
Compare
doronbehar
force-pushed
the
pkg/mlterm-wayland
branch
from
b633ffb
to
52016d5
Compare
|
With your permission, I'll merge this when CI is green, in the next few hours. |
|
We don't need to wait on aarch64-darwin CI, it's dog slow and I already tested it. |
|
I will note here that something went wrong with the compilation of the |
|
Fix in #232706 |
Motivation for this change
I wanted to be able to decoratively and elegantly enable or disable features, specifically wayland features.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes