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

fix: nushell language syntax update #1624

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Conversation

lorenzk
Copy link
Contributor

@lorenzk lorenzk commented Sep 1, 2023

Summary

Nushell integration is broken because of the deprecated let-env, see https://www.nushell.sh/commands/docs/let-env.html#frontmatter-title-for-removed for details.

Fixes: #1609

Other Information

There is already pull request #1610 that does not seem to be worked on at the moment.

Manual testing on my MacBook M1 worked. I tried running the automated test suite, but there were too many unrelated errors. I hope your CI will tell us whether I did everything right.

Linting worked.

@lorenzk lorenzk requested a review from a team as a code owner September 1, 2023 08:18
@lorenzk lorenzk changed the title Fix nushell integration fix: nushell integration Sep 1, 2023
@lorenzk
Copy link
Contributor Author

lorenzk commented Sep 1, 2023

I made a mistake that I only noticed after restarting my iTerm2. Fixed it in the third commit, but did not rebase or anything to keep things in historical order. Feel free to merge/squash however you like.

Copy link
Contributor

@hyperupcall hyperupcall left a comment

Choose a reason for hiding this comment

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

Thanks! When I test the changes, I recieve two errors: One I commented about and another about the ^asdf syntax. I'm not able to run CI here, but you can see the output from one of my forks. The only difference is that I squashed your changes.

About the ^asdf syntax, it seems that it is supposed to refer to the current module, but it does not seem to work anymore. I get the error:

# × External command failed
#      ╭─[/Users/runner/work/asdf/asdf/asdf.nu:98:1]
#   98 │
#   99 │         ^asdf plugin list $flags | lines | parse -r $template | str trim
#      ·          ──┬─
#      ·            ╰── command 'asdf' was not found but it exists in module 'asdf'; try importing it with `use`
#  100 │     }
#      ╰────

asdf.nu Outdated Show resolved Hide resolved
Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

Before we can merge this can you @lorenzk address the comment from @hyperupcall ?

Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

Thanks

@jthegedus jthegedus changed the title fix: nushell integration fix: nushell language syntax update Sep 11, 2023
@jthegedus jthegedus merged commit 0ddab5d into asdf-vm:master Sep 11, 2023
8 checks passed
@lorenzk
Copy link
Contributor Author

lorenzk commented Sep 11, 2023

Sorry for not replying earlier. You're awesome, thanks! Assuming brew to exist made no sense at all on my part.

@jthegedus
Copy link
Contributor

Sorry for not replying earlier. You're awesome, thanks! Assuming brew to exist made no sense at all on my part.

All good mate, you did the heavy lifting, much appreciated 🙇

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.

bug: Deprecated Call 'let-env' in asdf.nu
3 participants