Skip to content

fix(_comp_command_offset): Add support for complete -C #793

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 26, 2023
Merged

fix(_comp_command_offset): Add support for complete -C #793

merged 1 commit into from
Dec 26, 2023

Conversation

elyscape
Copy link
Contributor

@elyscape elyscape commented Sep 1, 2022

Fixes #631.

akinomyoga
akinomyoga requested changes Sep 1, 2022
View reviewed changes
Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

LGTM except for the following minor points.

Also, I think we can add test cases for the line continuation by the trailing \.

@akinomyoga akinomyoga mentioned this pull request Sep 1, 2022
Merged
@elyscape elyscape force-pushed the support-completion-commands branch from 8bdb305 to d5c5a6a Compare September 1, 2022 04:08
@elyscape
Copy link
Contributor Author

elyscape commented Sep 1, 2022

I wasn't able to figure out a way to test for completion after the newline is inserted. That being said, it does seem to function as expected, and I expect that it doesn't actually come up that often in practice.

@elyscape elyscape requested a review from akinomyoga September 1, 2022 04:16
akinomyoga
akinomyoga requested changes Sep 1, 2022
View reviewed changes
Copy link
Collaborator

@akinomyoga akinomyoga left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for updating! These are the remaining points:

@elyscape elyscape force-pushed the support-completion-commands branch from d5c5a6a to 5c505b2 Compare September 1, 2022 07:11
@elyscape
Copy link
Contributor Author

elyscape commented Sep 1, 2022

Requested changes applied! Let me know if there's anything else I should change.

@elyscape elyscape requested a review from akinomyoga September 1, 2022 07:14
@elyscape elyscape force-pushed the support-completion-commands branch 2 times, most recently from fd2e694 to e87a356 Compare September 1, 2022 08:00
@akinomyoga
Copy link
Collaborator

Thank you! Now I think everything is OK. Let's wait for @scop's check and decision!

@akinomyoga akinomyoga mentioned this pull request Sep 3, 2022
Closed
@elyscape elyscape force-pushed the support-completion-commands branch from e87a356 to b44e661 Compare September 7, 2022 06:26
@elyscape
Copy link
Contributor Author

elyscape commented Sep 7, 2022

I went ahead and rebased onto master to pick up the changes in #794.

@elyscape elyscape force-pushed the support-completion-commands branch from b44e661 to 2677dc3 Compare September 16, 2022 01:07
@elyscape
Copy link
Contributor Author

elyscape commented Sep 16, 2022 •
edited
Loading

Rebased onto master to handle changes from #797 and #799.

@elyscape elyscape force-pushed the support-completion-commands branch from 2677dc3 to 3a615b0 Compare September 16, 2022 01:11
@elyscape elyscape changed the title fix(_command_offset): Add support for complete -C fix(_comp_command_offset): Add support for complete -C Sep 16, 2022
@elyscape
Copy link
Contributor Author

elyscape commented Oct 3, 2022

Just checking in case this fell through the cracks.

scop
scop requested changes Nov 26, 2022
View reviewed changes
Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

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

Sorry for sitting on this for so long. Added a few comments + looks like we need another rebase.

@elyscape
Copy link
Contributor Author

elyscape commented Dec 2, 2022

I'll take a look this weekend.

@elyscape elyscape force-pushed the support-completion-commands branch 5 times, most recently from e328a12 to 25e745e Compare December 16, 2022 04:03
@elyscape elyscape requested a review from scop December 16, 2022 04:34
@elyscape
Copy link
Contributor Author

Rebased again and addressed comments. We should be good to go now.

@elyscape
Copy link
Contributor Author

@scop Just checking in on this.

@akinomyoga akinomyoga force-pushed the support-completion-commands branch from 25e745e to cc2f4e7 Compare September 24, 2023 10:39
@akinomyoga
Copy link
Collaborator

rebased.

@elyscape
Copy link
Contributor Author

@scop Pinging on this.

@akinomyoga akinomyoga force-pushed the support-completion-commands branch from cc2f4e7 to 4cb256c Compare December 24, 2023 20:21
akinomyoga
akinomyoga approved these changes Dec 24, 2023
View reviewed changes
Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

rebased & applied change ret => REPLY (#987)

scop
scop approved these changes Dec 26, 2023
View reviewed changes
Copy link
Owner

@scop scop 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 for your patience, this took way too long, my bad.

@scop scop merged commit 80450ca into scop:master Dec 26, 2023
@elyscape elyscape deleted the support-completion-commands branch April 10, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scop scop scop approved these changes

@akinomyoga akinomyoga akinomyoga approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use sudo with complete -C commands
3 participants
@elyscape @akinomyoga @scop