NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.34k stars 14.3k forks source link

xfce4-terminal: no sixel support #252845

Open gepbird opened 1 year ago

gepbird commented 1 year ago

Describe the bug

According to the xfce4-terminal v1.1.0 changelogs ,sixel is supported, but it doesn't work.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Install xfce.xfce4-terminal and lsix
  2. Run xfce4-terminal
  3. Inside the terminal run lsix

Expected behavior

Lsix should say sixel is supported.

Screenshots with actual result

Additional context

Maybe vte should be built with an additional flag, -Dsixel=true to fix this issue?

Notify maintainers

vte: @bobby285271 @romildo @muscaln xfce: @astsmtl @antono

Metadata

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.1.49, NixOS, 23.11 (Tapir), 23.11pre520402.e7f38be3775b`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.17.0`
 - channels(root): `"nixos, stable-23.05"`
 - channels(gep): `"nixpkgs"`
 - nixpkgs: `/home/gep/.nix-defexpr/channels/nixpkgs`
bobby285271 commented 1 year ago

This is not supported in stable vte release - https://github.com/GNOME/vte/commit/e0720853f59ebf54e022b4cd164de2d9b8a21fc1

linsui commented 1 year ago

vet unstable is also needed by blackbox. We can use the same vte unstable. https://github.com/NixOS/nixpkgs/pull/248682

bobby285271 commented 1 year ago

There is https://github.com/NixOS/nixpkgs/pull/224104 which is rejected previously, I personally don't use the feature so I can't comment much here 😂 (I imagine you can always do it in you personal nixos-config though...)

linsui commented 1 year ago

Yes, I put it in my overlays and it works well. Given that this feature is already used in terminals, I thought it's stable enough. @jtojnar Could you please reconsider the decision?

jtojnar commented 1 year ago

@linsui I am not aware of anything changing with regards to https://github.com/NixOS/nixpkgs/pull/224104#issuecomment-1492426531

linsui commented 1 year ago

@jtojnar What's your preferred methed to support features supported by upstream build in nixpkgs build? Is local override good to you?

jtojnar commented 1 year ago

I prefer enableFoo boolean input argument without an explicit attribute but note that the sixel feature is not supported by upstream.

linsui commented 1 year ago

Do you mean that we should add a enableSixel arg to terminals and use a overrided vte with sixel support when the arg is true?

note that the sixel feature is not supported by upstream.

The vte lib is not our upstream. The blackbox and xfce terminal devs are our upstream. The sixel feature is supported by the terminal packages.

jtojnar commented 1 year ago

If the terminal emulator claims to support sixel out of the box, I would just use overridden vte internally in the emulator derivation.

The sixel feature is supported by the terminal packages.

Then they are building on unstable foundations.

linsui commented 1 year ago

If the terminal emulator claims to support sixel out of the box, I would just use overridden vte internally in the emulator derivation.

Yes, that what I do for blackbox in https://github.com/NixOS/nixpkgs/pull/248682. Not sure about xfce terminal. Local override is good for one package but I thought it's better to use the same unstable vte everywhere.

jtojnar commented 1 year ago

Unless the development of the two projects is co-ordinated, each will probably be tested with a different vte commit so tracking them separately will better match the development model.