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

Update nushell crates #64

Merged
merged 1 commit into from
Oct 20, 2024
Merged

Update nushell crates #64

merged 1 commit into from
Oct 20, 2024

Conversation

diniamo
Copy link
Contributor

@diniamo diniamo commented Oct 19, 2024

Description of changes

Updates the nushell crates, and adjusts the code according to the breaking changes.

Relevant Issues

NixOS/nixpkgs#341647

@diniamo
Copy link
Contributor Author

diniamo commented Oct 19, 2024

No clue why that test fails

@AucaCoyan
Copy link
Contributor

AucaCoyan commented Oct 20, 2024

Hi! thank you for updating the deps.
Apparently the parser got updates since 0.88.1 and now it parses things a little different.

I test it on my pc and the result of the test ignore_comments is

        let expected = "# beginning of script comment
let one = 1
def my-func [
    param1:int # inline comment
]{ print(param1) 
}
myfunc (one )
# final comment";

(a little space difference in the line myfunc (one )
Modify that and should be good to go!

@diniamo
Copy link
Contributor Author

diniamo commented Oct 20, 2024

Yeah I got that far as well, but is that actually the result we want? I wouldn't want my function calls looking like that.

@AucaCoyan
Copy link
Contributor

Sure, it can (and should eventually) be fixed, but I prefer to merge this as is and then in another pr fix/improve the formatter logic and after change the tests that looks weird.
Isn't it better?

@fdncred
Copy link
Collaborator

fdncred commented Oct 20, 2024

ok, i'll land it if someone fixes the problem.

@fdncred fdncred merged commit decc88e into nushell:main Oct 20, 2024
5 checks passed
@diniamo
Copy link
Contributor Author

diniamo commented Oct 20, 2024

Well, I'm fine either way. I don't even use nu, I just wanted to get the development environment working (couldn't for 3 different reasons), and try it.

I might wait with updating the nixpkgs PR until someone fixes it though.

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.

3 participants