Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yazi: add testbed, adapt to 0.4.x changes #719

Merged
merged 7 commits into from
Jan 3, 2025

Conversation

xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Jan 1, 2025

Things done:


Fixes #604
Fixes #683

@xokdvium xokdvium force-pushed the dev/yazi-selected-copied branch from 87133b1 to fd5e9e7 Compare January 1, 2025 20:53
@trueNAHO
Copy link
Collaborator

trueNAHO commented Jan 1, 2025

From 88c04aa8f46a1e6b43f5cd6107f4ecbd725ea77f Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <[email protected]>
Date: Wed, 1 Jan 2025 18:52:19 +0300
Subject: [PATCH 1/3] yazi: add testbed

---
 modules/yazi/testbed.nix | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 modules/yazi/testbed.nix

diff --git a/modules/yazi/testbed.nix b/modules/yazi/testbed.nix
new file mode 100644
index 000000000..0c4467db2
--- /dev/null
+++ b/modules/yazi/testbed.nix
@@ -0,0 +1,32 @@
+{ pkgs, ... }:
+
+let
+  package = pkgs.yazi;
+in
+
+{
+  stylix.testbed.application = {
+    enable = true;
+    name = "yazi";
+    inherit package;
+  };
+
+  home-manager.sharedModules = [
+    {
+      programs.yazi = {
+        enable = true;
+        inherit package;
+      };
+
+      home.packages = [
+        # This has been changed in nixos-unstable: https://www.github.com/NixOS/nixpkgs/pull/354543
+        # TODO: Change to nerd-fonts.fira-mono with the next nixpkgs bump.
+        (pkgs.nerdfonts.override {
+          fonts = [
+            "FiraCode"
+          ];
+        })
+      ];
+    }
+  ];
+}

Thanks for adding a testbed. Could you update the nixpkgs input and already resolve the nerd-fonts issue?

From 940ab6fda94e9c6ce71414fda06233f36721dab8 Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <[email protected]>
Date: Wed, 1 Jan 2025 15:18:02 +0300
Subject: [PATCH 2/3] yazi: reverse marker_selected -> marker_copied

---
 modules/yazi/hm.nix | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/yazi/hm.nix b/modules/yazi/hm.nix
index 7cfe115d9..d17b86007 100644
--- a/modules/yazi/hm.nix
+++ b/modules/yazi/hm.nix
@@ -28,8 +28,8 @@
         preview_hovered = hovered;
         find_keyword = (mkFg base0B) // {bold = true;};
         find_position = mkFg base05;
-        marker_selected = mkSame base0B;
-        marker_copied = mkSame base0A;
+        marker_selected = mkSame base0A;
+        marker_copied = mkSame base0B;
         marker_cut = mkSame base08;
         tab_active = mkBoth base00 base0D;
         tab_inactive = mkBoth base05 base01;

LGTM.

From fd5e9e751b4545f28830a2935fc4ed5a4eb35799 Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <[email protected]>
Date: Wed, 1 Jan 2025 15:32:39 +0300
Subject: [PATCH 3/3] yazi: migrate highlight colors to mnemonics

This greatly improves readability and simplifies the process of reusing upstream
templates [1] in stylix modules.

Migration done with a simple bash script:

```bash
colors_base=("base08" "base09" "base0A" "base0B" "base0C" "base0D" "base0E" "base0F")
colors_name=("red" "orange" "yellow" "green" "cyan" "blue" "magenta" "brown")

for i in "${!colors_base[@]}"; do
  sed -i "s/${colors_base[i]}/${colors_name[i]}/g" modules/yazi/hm.nix
done

[1] https://github.com/yazi-rs/flavors/blob/main/scripts/catppuccin/template.toml

While this short-term solution simplifies the process of reusing upstream templates, it increases the inconsistencies in our code base. It might be best to wait for our saviour:

  • (4) Support base24 scheme 7 and Vim highlight groups 5 to resolve styling inconsistencies

-- NAHO, "Roadmap"

@xokdvium
Copy link
Contributor Author

xokdvium commented Jan 1, 2025

While this short-term solution simplifies the process of reusing upstream templates

It might be best to wait for our saviour:

Those seem quite orthogonal to each other? I think we can still greatly benefit from readability by using mnemonics right now. base16-nix also has support for base24 with bright- mnemonics, so that using them for base16 in no way detracts from the goal of supporting base24, which seems to be the agreed-upon target for the near future: #252 (comment), #252 (comment).

I'll be more than happy to come up with a base24 scheme, but I'd still use mnemonics to keep things sane.

Could you update the nixpkgs input and already resolve the nerd-fonts issue?

I guess in that case I can include fixes for #683 in this pr as well.

@trueNAHO
Copy link
Collaborator

trueNAHO commented Jan 1, 2025

While this short-term solution simplifies the process of reusing upstream templates

It might be best to wait for our saviour:

Those seem quite orthogonal to each other? I think we can still greatly benefit from readability by using mnemonics right now. base16-nix also has support for base24 with bright- mnemonics, so that using them for base16 in no way detracts from the goal of supporting base24, which seems to be the agreed-upon target for the near future: #252 (comment), #252 (comment).

I'll be more than happy to come up with a base24 scheme, but I'd still use mnemonics to keep things sane.

LGTM. Keep the changes.

Could you update the nixpkgs input and already resolve the nerd-fonts issue?

I guess in that case I can include fixes for #683 in this pr as well.

Yes.

@xokdvium xokdvium force-pushed the dev/yazi-selected-copied branch from fd5e9e7 to a03fb61 Compare January 1, 2025 22:20
@xokdvium xokdvium changed the title yazi: add testbed, reverse marker_selected -> marker_copied, migrate to mnemonics flake: bump nixpkgs, yazi: add testbed, adapt to 0.4.x changes Jan 1, 2025
Copy link
Collaborator

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't want me to squash merge this PR, slightly change the commit structure as follows.

Squash the

28f6c00 yazi: remove 'x-' prefix from mimetypes
d276edb yazi: migrate select -> pick
7d14c78 yazi: permissions_{t,r,w,x,s} -> perm_{type,read,write,exec,sep}
a03fb61 yazi: replace mode_* with mode.*_{main,alt} settings

commits into

5c22a76 flake: bump nixpkgs

because the 0.4.x breaking changes are introduced by the nixpkgs bump.

Then the final order could look like:

e79f210 yazi: add testbed
4f956ab yazi: reverse marker_selected -> marker_copied
79cf0c1 yazi: migrate highlight colors to mnemonics
<HASH>  <SQUASHED_COMMIT>

I don't mind the order of the last two commits.

You might also want to add the

Closes: https://github.com/danth/stylix/issues/604

and

Closes: https://github.com/danth/stylix/issues/683

tags to the appropriate commits

Otherwise, the PR looks great.

If you don't care about the full PR squash, I could simply merge it as:

yazi: add testbed, resolve 0.4.0 breaking changes, and improve names

Add a testbed and resolve the 0.4.0 breaking changes [2] [3] [4].

Migrate the highlight colors to mnemonics with the following script to
improve readability and simplify the process of reusing upstream
templates [4]:

    colors_base=(
      "base08" "base09" "base0A" "base0B" "base0C" "base0D" "base0E"
      "base0F"
    )

    colors_name=(
      "red" "orange" "yellow" "green" "cyan" "blue" "magenta" "brown"
    )

    for i in "${!colors_base[@]}"; do
      sed \
        --in-place \
        "s/${colors_base[i]}/${colors_name[i]}/g" \
        modules/yazi/hm.nix
    done

[1]: https://github.com/sxyazi/yazi/issues/1772
[2]: https://github.com/sxyazi/yazi/pull/1927
[3]: https://github.com/sxyazi/yazi/pull/1953
[4]: https://github.com/yazi-rs/flavors/blob/main/scripts/catppuccin/template.toml

Closes: https://github.com/danth/stylix/issues/604
Closes: https://github.com/danth/stylix/issues/683
Link: https://github.com/danth/stylix/pull/719

Reviewed-by: NAHO <[email protected]>

@xokdvium
Copy link
Contributor Author

xokdvium commented Jan 2, 2025

If you don't care about the full PR squash, I could simply merge it as:

Squash-merge is fine by me. Separate commits are primarily for ease of review. I'll drop the nixpkgs bump and rebase, since 6eb0597 already bumps nixpkgs and it's good to merge as is.

You might also want to add the tags to the appropriate commits

Personally, I'm not a huge fan of doing this, since force pushes with such commit messages tend to cause repeated mentions in the referenced issues/prs. Having the Fixes https://github.com/danth/stylix/issues/604 in PR description is enough to resolve the issues when the pr gets merged.

That's also why the https://www.github.com/sxyazi/yazi/issues/1772 being prefixed with www.. That's one workaround to prevent irrelevant mentions and unnecessary mentions with backreferences (see the comment).

That's just my my two cents.

This greatly improves readability and simplifies the process of reusing upstream
templates [1] in stylix modules.

Migration done with a simple bash script:

```bash
colors_base=("base08" "base09" "base0A" "base0B" "base0C" "base0D" "base0E" "base0F")
colors_name=("red" "orange" "yellow" "green" "cyan" "blue" "magenta" "brown")

for i in "${!colors_base[@]}"; do
  sed -i "s/${colors_base[i]}/${colors_name[i]}/g" modules/yazi/hm.nix
done
```

[1] https://github.com/yazi-rs/flavors/blob/main/scripts/catppuccin/template.toml
Breaking change for 0.4.0 release, according to the migration guide [1]:
> 1.2 - Renamed the select component to pick component

[1]: https://www.github.com/sxyazi/yazi/issues/1772
Yet another breakage from [1] for 0.4.0 changes. This fixes the unreadable
status bar issue.

[1] https://www.github.com/sxyazi/yazi/pull/1953
@xokdvium xokdvium force-pushed the dev/yazi-selected-copied branch from a03fb61 to d27a2b1 Compare January 2, 2025 20:11
@xokdvium xokdvium changed the title flake: bump nixpkgs, yazi: add testbed, adapt to 0.4.x changes yazi: add testbed, adapt to 0.4.x changes Jan 2, 2025
Copy link
Collaborator

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't care about the full PR squash, I could simply merge it as:

Squash-merge is fine by me. Separate commits are primarily for ease of review.

Yes, they made reviewing a lot easier and faster. Thanks.

You might also want to add the tags to the appropriate commits

Personally, I'm not a huge fan of doing this, since force pushes with such commit messages tend to cause repeated mentions in the referenced issues/prs. Having the Fixes https://github.com/danth/stylix/issues/604 in PR description is enough to resolve the issues when the pr gets merged.

That's also why the https://www.github.com/sxyazi/yazi/issues/1772 being prefixed with www.. That's one workaround to prevent irrelevant mentions and unnecessary mentions with backreferences (see the comment).

That's just my my two cents.

I did not know there was a way to circumvent this pesky GitHub feature. Thanks for sharing!

@trueNAHO trueNAHO merged commit 0ce2a52 into danth:master Jan 3, 2025
40 checks passed
pinarruiz pushed a commit to pinarruiz/stylix that referenced this pull request Jan 4, 2025
…anth#719)

Add a testbed and resolve the 0.4.0 breaking changes [2] [3] [4].

Migrate the highlight colors to mnemonics with the following script to
improve readability and simplify the process of reusing upstream
templates [4]:

    colors_base=(
      "base08" "base09" "base0A" "base0B" "base0C" "base0D" "base0E"
      "base0F"
    )

    colors_name=(
      "red" "orange" "yellow" "green" "cyan" "blue" "magenta" "brown"
    )

    for i in "${!colors_base[@]}"; do
      sed \
        --in-place \
        "s/${colors_base[i]}/${colors_name[i]}/g" \
        modules/yazi/hm.nix
    done

[1]: sxyazi/yazi#1772
[2]: sxyazi/yazi#1927
[3]: sxyazi/yazi#1953
[4]: https://github.com/yazi-rs/flavors/blob/main/scripts/catppuccin/template.toml

Closes: danth#604
Closes: danth#683
Link: danth#719

Reviewed-by: NAHO <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yazi: unreadable status bar yazi switched yank and selected color
2 participants