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

refactor(nushell): update commandline syntax, closes #1733 #1735

Merged
merged 2 commits into from Feb 20, 2024

Conversation

stevenxxiu
Copy link
Contributor

This should be merged when the next version of Nushell releases, where it'll change the commandline syntax.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

@fdncred
Copy link

fdncred commented Feb 19, 2024

The next nushell release is scheduled for 5th of March.

@arcuru
Copy link
Sponsor Contributor

arcuru commented Feb 19, 2024

Is it possible to check the Nushell version and support both versions?

@stevenxxiu
Copy link
Contributor Author

I could do that, but this adds the additional burden of removing the code later down the track.

I feel like Nushell is still unstable, so people should update regularly anyway.

@fdncred
Copy link

fdncred commented Feb 19, 2024

Nushell does populate $env.NU_VERSION for an easy way to check the nushell version running. One can also do nu --version and get the same version string.

@arcuru
Copy link
Sponsor Contributor

arcuru commented Feb 19, 2024

Nushell itself is apparently going to support the old way for 1 release before deprecating. Some of Atuin's users may be on platforms where they aren't running the absolute latest Nushell.

This change as written will cause Atuin to deal with the Nushell breakage, as anyone on older versions of Nushell will not get an appropriate warning message and will think Atuin has the bug. That will be far more burden than removing the code down the line.

Nushell having breaking changes is fine, but all programs trying to work with Nushell, like Atuin, will be trying to support older versions where possible. Having a point in time where a user has to update both Nushell and Atuin at exactly the same time is unreasonable.

@stevenxxiu
Copy link
Contributor Author

True. I suppose Atuin and Nushell won't always update at similar times.

I've added backwards compatibility for Nushell <= v0.90.

@stevenxxiu stevenxxiu force-pushed the refactor/nushell-commandline branch 3 times, most recently from 6af6043 to f82189a Compare February 19, 2024 15:53
@stevenxxiu
Copy link
Contributor Author

With the code being backwards compatible, this could probably be reviewed and merged before the Nushell release. If there's not more breaking changes on this front. @ellie @fdncred

@ellie ellie mentioned this pull request Feb 20, 2024
2 tasks
Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

@ellie ellie merged commit 1c29702 into atuinsh:main Feb 20, 2024
15 checks passed
@stevenxxiu stevenxxiu deleted the refactor/nushell-commandline branch March 13, 2024 15:58
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

4 participants