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

jujutsu: fix completions commands #5016

Conversation

cdmistman
Copy link

Description

starting in jujutsu 0.14.0, jujutsu prefers jj util completion <shell> instead of jj util completion --<shell>. When the flag variation is used, jujutsu prints the following on shell startup:

Warning: `jj util completion --zsh` will be removed in a future version, and this will be a hard error
Hint: Use `jj util completion zsh` instead

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

@shikanime

@teto
Copy link
Collaborator

teto commented Feb 10, 2024

this looks ok but can it be safely enabled before 0.14 ? the HM flake still points at 0.13

@cdmistman
Copy link
Author

yeah this is a bit pre-emptive. this requires jj >= 0.14.0. once the flake nixpkgs input has 0.14.0 this should be merged

@ambroisie
Copy link
Contributor

I don't quite understand how the module came to explicitly call this, the package already took care of installing the completion files which should be picked up by home-manager when it manages the shell conifguration.

And this was fixed when the update was merged upstream ;-) (NixOS/nixpkgs@59b0ff3).

@rycee
Copy link
Member

rycee commented Feb 12, 2024

Nice, yeah if the package provides the completion files then there should be no need to also have them here. So in principle it should be OK to remove the shell init configuration and remove the "enable integration" options (using mkRemovedOptionModule).

@shikanime
Copy link
Contributor

I wasn't even aware that Nixpkgs already handle shell completions, but does it work with non-NixOS installations ?

@cdmistman
Copy link
Author

ok, closing and another PR can be opened to remove the completions later

@cdmistman cdmistman closed this Feb 15, 2024
jcf added a commit to jcf/home-manager that referenced this pull request Feb 19, 2024
Completion is installed by the jujutsu package itself, making this
additional setup redundant.

nix-community#5016
nix-community#5037 (review)
nix-community#5016 (comment)
jcf added a commit to jcf/home-manager that referenced this pull request Feb 19, 2024
Completion is installed by the jujutsu package itself, making this
additional setup redundant.

nix-community#5016 (comment)
nix-community#5037 (review)
rycee pushed a commit to jcf/home-manager that referenced this pull request Feb 20, 2024
Completion is installed by the jujutsu package itself, making this
additional setup redundant.

nix-community#5016 (comment)
nix-community#5037 (review)
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.

None yet

5 participants