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

completion: teach commands about files #4887

Merged
merged 1 commit into from
Nov 28, 2024
Merged

completion: teach commands about files #4887

merged 1 commit into from
Nov 28, 2024

Conversation

senekor
Copy link
Contributor

@senekor senekor commented Nov 16, 2024

This is heavily based on Benjamin Tan's fish completions:
https://gist.github.com/bnjmnt4n/9f47082b8b6e6ed2b2a805a1516090c8

Some differences include:

  • The end of a --from, --to range is also considered.
  • jj log is not completed (yet). It has a different --revisions argument
    that requires some special handling.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

cli/src/commands/diff.rs Outdated Show resolved Hide resolved
@senekor senekor force-pushed the remo/vxqolomwrqtw branch 4 times, most recently from 4b076bb to c3335a6 Compare November 18, 2024 12:23
cli/src/complete.rs Outdated Show resolved Hide resolved
cli/src/complete.rs Outdated Show resolved Hide resolved
cli/src/complete.rs Outdated Show resolved Hide resolved
cli/src/complete.rs Outdated Show resolved Hide resolved
@senekor
Copy link
Contributor Author

senekor commented Nov 20, 2024

@yuja are you ok with the parse module in the middle of the file, tests strewn about? You asked me to move tests to the bottom in another PR. What do you prefer, should I move the parse module to a separate file?

@senekor senekor force-pushed the remo/vxqolomwrqtw branch 2 times, most recently from ce5fd3c to d82e1dc Compare November 20, 2024 19:53
cli/tests/test_completion.rs Outdated Show resolved Hide resolved
@senekor senekor force-pushed the remo/vxqolomwrqtw branch 2 times, most recently from f7c52c4 to 12d96d4 Compare November 20, 2024 21:30
@yuja
Copy link
Contributor

yuja commented Nov 21, 2024

@yuja are you ok with the parse module in the middle of the file, tests strewn about? You asked me to move tests to the bottom in another PR. What do you prefer, should I move the parse module to a separate file?

Suppose it is a temporary workaround for clap-rs/clap#5784, I don't mind if the parse module had an unusual layout. I would move mod parse to the bottom (before the tests module) and move tests to the outer mod tests, but that's just my personal preference.

@senekor senekor force-pushed the remo/vxqolomwrqtw branch 3 times, most recently from 697fd99 to d295af2 Compare November 27, 2024 17:54
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Thanks! This LGTM, but please give Yuya a chance to review since he's already reviewed an earlier version of it.

cli/src/commands/commit.rs Outdated Show resolved Hide resolved
cli/src/complete.rs Outdated Show resolved Hide resolved
cli/src/complete.rs Outdated Show resolved Hide resolved
This is heavily based on Benjamin Tan's fish completions:
https://gist.github.com/bnjmnt4n/9f47082b8b6e6ed2b2a805a1516090c8

Some differences include:
- The end of a `--from`, `--to` ranges is also considered.
- `jj log` is not completed (yet). It has a different `--revisions` argument
  that requires some special handling.
@senekor senekor merged commit 5fcc549 into main Nov 28, 2024
31 checks passed
@senekor senekor deleted the remo/vxqolomwrqtw branch November 28, 2024 14:34
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