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-unwrapped: build new yazi-cli tool #321937

Merged
merged 2 commits into from
Jul 3, 2024
Merged

Conversation

eljamm
Copy link
Contributor

@eljamm eljamm commented Jun 23, 2024

Description of changes

See sxyazi/yazi#914

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@eljamm
Copy link
Contributor Author

eljamm commented Jun 23, 2024

Result of nixpkgs-review pr 321937 run on x86_64-linux 1

2 packages built:
  • yazi
  • yazi-unwrapped

@eljamm
Copy link
Contributor Author

eljamm commented Jun 23, 2024

Tested the tool a little bit following the yazi DDS docs page and it seems to work.

Testing steps

  • Open yazi and enter a subshell
  • Find out the instance ID using echo $YAZI_ID. You can exit the yazi subshell, now.
  • In another shell, invoke a command to the instance using the cli. For example, to change the directory:
./result/bin/ya pub "1719138555289021" dds-cd --str "/tmp"
  • Back in the yazi instance, you'll notice that the working directory has changed

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jun 23, 2024
Copy link
Member

@XYenon XYenon left a comment

Choose a reason for hiding this comment

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

Yazi the wrapper also need to be changed to make ya available.

@@ -25,6 +25,8 @@ rustPlatform.buildRustPackage rec {

env.YAZI_GEN_COMPLETIONS = true;

cargoBuildFlags = ["-p" "yazi-fm" "-p" "yazi-cli"];
Copy link
Member

Choose a reason for hiding this comment

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

I have added it to default-members, so we can remove this next version.

https://github.com/sxyazi/yazi/pull/944/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So do you think we should just wait for the next release or do we add a message like this:

  # remove in the next release
  cargoBuildFlags = ["-p" "yazi-fm" "-p" "yazi-cli"];

@eljamm
Copy link
Contributor Author

eljamm commented Jun 24, 2024

Yazi the wrapper also need to be changed to make ya available.

Since the wrapper changed in 321076, I didn't want to modify it here until it was merged to avoid conflicts.

Would it have been better if I opened this as a draft, perhaps?

eljamm added 2 commits July 3, 2024 09:26
In addition to the main program, there is a new CLI tool, now:
sxyazi/yazi#914
@eljamm
Copy link
Contributor Author

eljamm commented Jul 3, 2024

The wrapper PR was merged so I've added the cli tool to the wrapper, tested again, and everything appears to be working.

I've also added a note to remove the cargo flags in the next release.

@ofborg ofborg bot requested a review from XYenon July 3, 2024 09:04
@SuperSandro2000 SuperSandro2000 merged commit f63739e into NixOS:master Jul 3, 2024
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants