Skip to content

fix(completion): file completion for redirection target#16831

Merged
ysthakur merged 3 commits into
nushell:mainfrom
blindFS:fix_redirection_completion
Oct 12, 2025
Merged

fix(completion): file completion for redirection target#16831
ysthakur merged 3 commits into
nushell:mainfrom
blindFS:fix_redirection_completion

Conversation

@blindFS
Copy link
Copy Markdown
Contributor

@blindFS blindFS commented Oct 8, 2025

Fixes #16827

Release notes summary - What our users need to know

Added file completions for command redirections (o>>, e>, ...)

Tasks after submitting

Comment thread crates/nu-cli/src/completions/completer.rs Outdated
@blindFS blindFS marked this pull request as draft October 8, 2025 22:05
@blindFS blindFS force-pushed the fix_redirection_completion branch from 65c6eff to a9ddcf5 Compare October 8, 2025 22:18
@blindFS blindFS force-pushed the fix_redirection_completion branch from a9ddcf5 to 364c411 Compare October 8, 2025 23:32
@blindFS blindFS marked this pull request as ready for review October 8, 2025 23:34
Copy link
Copy Markdown
Member

@ysthakur ysthakur left a comment

Choose a reason for hiding this comment

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

LGTM. Hopefully we don't run into any problems with the removal of the space splitting thing, but we can fix that later

}

/// Helper function to extract file-path expression from redirection target
fn check_redirection_target(target: &RedirectionTarget, pos: usize) -> Option<&Expression> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick: I feel like extract would be better than check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was named extract_xxx initially LoL. I don't remember what made me change my mind.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably because I later moved the position checking to this function.

@ysthakur ysthakur merged commit 8229ccb into nushell:main Oct 12, 2025
16 checks passed
@github-actions github-actions Bot added this to the v0.108.0 milestone Oct 12, 2025
@ysthakur ysthakur added A:completions Issues related to tab completion notes:additions Noted in "Additions" section labels Oct 12, 2025
@ysthakur
Copy link
Copy Markdown
Member

For the release notes summary, something like "Added file completions for command redirections" should be enough

@blindFS blindFS deleted the fix_redirection_completion branch October 13, 2025 03:16
@Bahex Bahex added the notes:ready Indicates Ready for Release notes label Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:completions Issues related to tab completion notes:additions Noted in "Additions" section notes:ready Indicates Ready for Release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File completion on the command line does not work when redirecting output

3 participants