Make which-support feature non-optional#13125
Merged
fdncred merged 3 commits intonushell:mainfrom Jun 13, 2024
Merged
Conversation
ysthakur
reviewed
Jun 11, 2024
Member
ysthakur
left a comment
There was a problem hiding this comment.
Nice job finding this, LGTM but I'll let someone else with more knowledge of cargo approve. Just had one thing.
Contributor
|
I asked about this a while back and someone was speculating that the reason it was feature-gated was because of it not working on other operating systems, maybe one of the BSDs but I'm not sure exactly. Maybe @devyn can test on a BSD release to see if there are problems? |
Contributor
|
@fdncred I have not had any trouble compiling or using |
Contributor
|
ok, let's try it. thanks! |
Merged
fdncred
pushed a commit
that referenced
this pull request
Jun 29, 2024
Recommend holding until after #13125 is fully digested and *possibly* until 0.96. # Description Fixes one of the issues described in #13125 The `do` signature included a `SyntaxShape:Any` as one of the possible first-positional types. This is incorrect. `do` only takes a closure as a positional. This had the result of: 1. Moving what should have been a parser error to evaluation-time ## Before ```nu > do 1 Error: nu::shell::cant_convert × Can't convert to Closure. ╭─[entry #26:1:4] 1 │ do 1 · ┬ · ╰── can't convert int to Closure ╰──── ``` ## After ```nu > do 1 Error: nu::parser::parse_mismatch × Parse mismatch during operation. ╭─[entry #5:1:4] 1 │ do 1 · ┬ · ╰── expected block, closure or record ╰──── ``` 2. Masking a bad test in `std assert` This is a bit convoluted, but `std assert` tests included testing `assert error` to make sure it: * Asserts on bad code * Doesn't assert on good code The good-code test was broken, and was essentially bad-code (really bad-code) that wasn't getting caught due to the bad signature. Fixing this resulted in *parse time* failures on every call to `test_asserts` (not something that particular test was designed to handle. This PR also fixes the test case to properly evaluate `std assert error` against a good code path. # User-Facing Changes * Error-type returned (possible breaking change?) # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting N/A
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Removes the
which-supportcargo feature and makes all of its feature-gated code enabled by default in all builds. I'm not sure why this one command is gated behind a feature. It seems to be a relic of older code where we had features for what seems like every command.