Skip to content

Conversation

@cablehead
Copy link
Contributor

@cablehead cablehead commented Nov 4, 2025

The --unix-socket flag in all HTTP commands now supports tilde expansion, allowing paths like ~/my-service.sock to work correctly.

Copy link
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.

Thanks for this. Haven't studied the completer.rs changes too closely, but this should be split into two PRs: one for the completion changes and one for the http get changes. It doesn't seem like either change is dependent on the other.

@blindFS
Copy link
Contributor

blindFS commented Nov 5, 2025

Thanks for the fix. I also noticed this problem while reviewing #16859

There're hard conflicts between these 2 PRs, and I'm planning on a shared completer for both positional args and flag values after #16859 .

Hopefully we can get all those things done before the next release.

So I think we'll on-hold merging of this one, if you don't mind.

@ysthakur
Copy link
Member

ysthakur commented Nov 5, 2025

Yeah I'd recommend modifying this PR to only modify http get

@cablehead
Copy link
Contributor Author

cablehead commented Nov 5, 2025

Thank you @blindFS & @ysthakur for giving this a look over!

@blindFS just to clarify: the change you are planning will also provide a default completion for flag arguments which have a File and Directory shape?

amazing if so! and yea, if that is the case, I'll just make this PR handle expanding the path for the http commands

@blindFS
Copy link
Contributor

blindFS commented Nov 5, 2025

@blindFS just to clarify: the change you are planning will also provide a default completion for flag arguments which have a File and Directory shape?

Yes, just as what it does for positional/normal arguments now.

And the benefit is that if we add support for other types, e.g. bool, list, record (consider a snippet like expansion), in the future. It will work automatically for both flag values and normal args.

@cablehead
Copy link
Contributor Author

And the benefit is that if we add support for other types, e.g. bool, list, record (consider a snippet like expansion), in the future. It will work automatically for both flag values and normal args.

amazing 🙌

Keep only the tilde expansion for --unix-socket flag.
The general filepath completion feature will be proposed separately.
@cablehead cablehead changed the title feat(completion): add filepath completion for flags fix(http): add tilde expansion for --unix-socket paths Nov 5, 2025
@cablehead cablehead requested a review from ysthakur November 5, 2025 03:18
@cablehead
Copy link
Contributor Author

@blindFS , @ysthakur lemme know if this looks good to you now

@blindFS
Copy link
Contributor

blindFS commented Nov 6, 2025

Would be nice to have a test case for the socket path expansion.

@cablehead cablehead marked this pull request as draft November 6, 2025 22:50
@cablehead cablehead marked this pull request as ready for review November 7, 2025 20:10
@cablehead
Copy link
Contributor Author

@blindFS test added.

Copy link
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, thanks. Just make sure to add a release note summary to your PR

@ysthakur ysthakur merged commit 75c1675 into nushell:main Nov 8, 2025
16 checks passed
@github-actions github-actions bot added this to the v0.109.0 milestone Nov 8, 2025
WindSoilder pushed a commit that referenced this pull request Nov 13, 2025
…letion results (#17006)

First step of
[this](#16859 (comment)),
mainly to reuse the type-based completions for flag values, as reported
in #16977 .

And some complement test cases.

@WindSoilder @ysthakur 

## Release notes summary - What our users need to know

Fixed a bug where path/glob/directory typed flags don't get completions.

## Tasks after submitting
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