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

Input output checking #9680

Merged
merged 6 commits into from Jul 14, 2023
Merged

Input output checking #9680

merged 6 commits into from Jul 14, 2023

Conversation

sophiajt
Copy link
Member

@sophiajt sophiajt commented Jul 13, 2023

Description

This PR tights input/output type-checking a bit more. There are a lot of commands that don't have correct input/output types, so part of the effort is updating them.

This PR now contains updates to commands that had wrong input/output signatures. It doesn't add examples for these new signatures, but that can be follow-up work.

User-Facing Changes

BREAKING CHANGE BREAKING CHANGE

This work enforces many more checks on pipeline type correctness than previous nushell versions. This strictness may uncover incompatibilities in existing scripts or shortcomings in the type information for internal commands.

Tests + Formatting

After Submitting

@sophiajt sophiajt marked this pull request as ready for review July 14, 2023 00:40
@sophiajt sophiajt added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Jul 14, 2023
@sophiajt sophiajt merged commit 786ba3b into nushell:main Jul 14, 2023
19 checks passed
@sophiajt sophiajt deleted the input_output_checking branch July 14, 2023 03:20
amtoine added a commit to amtoine/zellij-layouts that referenced this pull request Jul 14, 2023
`append` only accepts lists with nushell/nushell#9680
sholderbach added a commit that referenced this pull request Jul 24, 2023
# Description
Reallow the commands that take cellpaths as rest parameters to operate
on table input data.

Went through all commands returned by

```
scope commands |
  filter { |cmd| $cmd.signatures |
    values |
    any {|sig| $sig |
      any {|$sig| $sig.parameter_type == rest and $sig.syntax_shape ==
cellpath }
    }
  } | get name
```

Only exception to that was `is-empty` that returns a bool.
# User-Facing Changes
Same table operations as in `0.82` should still be possible
Mitigates effects of #9680
@mati865
Copy link

mati865 commented Jul 26, 2023

I think this might be source of regression in zoxide support for Nu: ajeetdsouza/zoxide#595 ajeetdsouza/zoxide#596
rest is declared as [...rest:string] so it's an array but when loading the script Nu treats it as a string and shows an error.

@amtoine
Copy link
Member

amtoine commented Jul 26, 2023

@mati865
yeah, we have an issue with rest parameters and type checking 😕

this has been reported independently in

sholderbach added a commit that referenced this pull request Jul 26, 2023
# Description
The same procedure as for #9778 repeated for records.

# User-Facing Changes
Commands that directly supported applying their work directly to record
fields via cell paths, that worked before #9680 will now work again

# Tests + Formatting
Tried to limit the need to add new `.allow_variants_without_examples()`
by adjusting or adding tests to also use some records with access.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants