Remove usages of Call::positional_nth
#12871
Merged
Conversation
This file contains 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
WindSoilder
reviewed
May 15, 2024
| pub fn positional_nth(&self, i: usize) -> Option<&Expression> { | ||
| self.positional_iter().nth(i) | ||
| } | ||
|
|
||
| // TODO this method is never used. Delete? | ||
| pub fn positional_nth_mut(&mut self, i: usize) -> Option<&mut Expression> { |
There was a problem hiding this comment.
I'm not sure if we should remove it. It seems like a useful method and can be used inside plugin.
There was a problem hiding this comment.
Disagree, we should have a minimal stable interface for the plugins now, and can add things back if there is a proven need.
I think the semantic meaning of mutating Expressions of a Call is weird (bar mem::take but only if proven useful). As a plugin you should only concern yourself with the resulting Values and associated Span.
There was a problem hiding this comment.
I don't believe Call is exposed to plugins. Rather, plugins use the EvaluatedCall type.
Although, it looks like Call does appear in the signature of EvaluatedCall::try_from_call.
sholderbach
added
the
pr:api-change
FilipAndersson245
pushed a commit
to FilipAndersson245/nushell
that referenced
this pull request
May 18, 2024
# Description Following from nushell#12867, this PR replaces usages of `Call::positional_nth` with existing spans. This removes several `expect`s from the code. Also remove unused `positional_nth_mut` and `positional_iter_mut`
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
Following from #12867, this PR replaces usages of
Call::positional_nthwith existing spans. This removes severalexpects from the code.