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

[docs] Simplify Nushell integration #5844

Closed
wants to merge 1 commit into from

Conversation

texastoland
Copy link
Contributor

@texastoland texastoland commented Mar 16, 2024

Description

Simplifies Nushell integration instructions (docs only) in the following ways:

  • Saves the script to a standard cross-platform path
  • Just use starship.nu instead of arbitrary path
  • Provides runnable commands
  • Explains why it's necessary
  • Explains how to update

Motivation and Context

Motivated by jdx/mise-docs#46 for consistency between tools.

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly (N/A).

Motivated by jdx/mise-docs#46 for consistency between tools.
Copy link
Member

@davidkna davidkna left a comment

Choose a reason for hiding this comment

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

It hasn't been more stable lately, but nushell has had many breaking changes and our init script also gets updated now and then, so I would like to avoid having the init script static at this time.

@texastoland
Copy link
Contributor Author

texastoland commented Mar 16, 2024

I would like to avoid having the init script static at this time.

It was already static. Here's the relevant diff:

-   starship init nu | save -f ~/.cache/starship/init.nu
+   starship init nu | save $starship_path --force

I included a link to explain why it's required:

+   Since Nu does [not support `eval`](https://www.nushell.sh/book/how_nushell_code_gets_run.html#eval-function) you must save the initialization script:

@davidkna
Copy link
Member

FWIW, it is not necessarily static between different versions of starship (or with different starship binary locations).

More importantly, if the file doesn't update itself, it will make handling user issues more difficult and will cause additional strife on breaking nushell updates. Given all that, I'd rather not accept this PR.

@davidkna davidkna closed this Mar 16, 2024
@texastoland
Copy link
Contributor Author

texastoland commented Mar 16, 2024

@davidkna There's a misunderstanding. I didn't change that recommendation. It is and always was "static" because Nu has never supported any eval-like functionality (doc link above). That confusion is why my PR exists.

  • Saves the script to a standard cross-platform path
  • Just use starship.nu instead of arbitrary path
  • Provides runnable commands
  • Explains why it's necessary
  • Explains how to update

These changes explain the conceptual reason and simplify installation for Nu users. I started a thread on your Discord server for clarification.

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

2 participants