gpg.fish refactor#6139
Closed
LawAbidingCactus wants to merge 3 commits into
fish-shell:masterfish-shell/fish-shell:masterfrom
LawAbidingCactus:gpg-fish-refactorCopy head branch name to clipboard
Closed
gpg.fish refactor#6139LawAbidingCactus wants to merge 3 commits intofish-shell:masterfish-shell/fish-shell:masterfrom LawAbidingCactus:gpg-fish-refactorCopy head branch name to clipboard
LawAbidingCactus wants to merge 3 commits into
fish-shell:masterfish-shell/fish-shell:masterfrom
LawAbidingCactus:gpg-fish-refactorCopy head branch name to clipboard
Conversation
zanchey
requested changes
Sep 25, 2019
zanchey
left a comment
Member
There was a problem hiding this comment.
As a general overview, I don't think this is the right approach - there is lots of duplicated code which I think could be simplified.
I wonder about:
- breaking gpg2 completions out into
__fish_complete_gpg2, which takes a__fish_complete_gpg2_commandargument, and which uses$__fish_complete_gpg2_commandinstead ofgpg2everywhere (in functions and completion arguments - in
gpg2.fish, call__fish_complete_gpg_command gpg2 - in
gpg.fish, check ifgpgis 2.x, and if so call__fish_complete_gpg2 gpg; otherwise use the gpg1 completions
Would that work?
ridiculousfish
added a commit
that referenced
this pull request
Oct 6, 2019
Revert "gut gpg.fish/gpg1.fish/gpg2.fish; migrate functionality to __fish_complete_gpg.fish" This reverts commit d558218. Revert "break version-specific completions out into independent function;" This reverts commit 9160e77. Revert "split gpg2- and gpg1-specific completions to conditional block" This reverts commit a069b95.
Member
|
I merged this but then saw the comments, so reverted the merge. |
Contributor
Author
|
@ridiculousfish: Yeah, sorry-- I implemented @zanchey's idea for deduplicating redundant completion code and decided to see if I could take it a little further. It's not ready yet, since I haven't had time to work on it lately. |
Contributor
Author
|
I've rewritten the PR to deduplicate redundant code to the fullest extent possible. The contextual completion functions, along with the GPG completions themselves, have been moved to `share/fish/functions/`. This allows `__fish_complete_gpg` to be called from the actual completion files to conditionally generate completions from the GnuPG version and argument given. This also allows completions for `gpg1` (as in, a binary called `gpg1`) to be easily generated, without adding redundant completion code. Some commenting is required but I'm otherwise done.
…On Sun, Oct 6, 2019 at 19:36, ridiculousfish ***@***.***> wrote:
I merged this but then saw the comments, so reverted the merge.505793
—
You are receiving this because you authored the thread.
Reply to this email directly, [view it on GitHub](#6139?email_source=notifications&email_token=AB7UAJG2MOEU5BTEOI4ORR3QNI423A5CNFSM4I2FWOMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAOSEHY#issuecomment-538780191), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AB7UAJCX5RSPUC6QASKLKK3QNI423ANCNFSM4I2FWOMA).
|
Member
|
note zanchey is on vacation so this may sit a while. If you are keen to have this merged before their return let me know. |
zanchey
approved these changes
Oct 20, 2019
zanchey
left a comment
Member
There was a problem hiding this comment.
Looks great; one comment-related change but otherwise good to go.
Contributor
|
Merged as 21a6a19, thanks for all the work! |
Member
|
Thanks for merging @krobelus, I was hoping to get to this today. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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
Refactors
gpg.fishto conditionally activate completions based ongpgversion, as discussed in #6062:TODOs: