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

revisit gentoo-related completions #4758

Merged
merged 11 commits into from Mar 23, 2018
Merged

revisit gentoo-related completions #4758

merged 11 commits into from Mar 23, 2018

Conversation

drwilly
Copy link
Contributor

@drwilly drwilly commented Feb 28, 2018

  • add completion for emaint
  • improve completion for emerge
    still incomplete, but an impovement regardless
  • improve completion for equery
  • add gentoo/portage-related auxiliary functions

- add completion for emaint
- improve completion for emerge
  still incomplete, but an impovement regardless
- improve completion for equery
- add gentoo/portage-related auxiliary functions
@@ -0,0 +1,3 @@
function __fish_portage_print_installed_pkgs --description 'Print all installed packages'
find /var/db/pkg -mindepth 2 -maxdepth 2 -type d -printf '%P\n' | sed 's/-[0-9][0-9.]*.*$//' | sort -u
Copy link
Member

Choose a reason for hiding this comment

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

fish will take care of the deduplication for you - no need to use sort.

Also we prefer our builtins over external commands like sed. Something like

find ... | string replace -r -- '-[0-9][0-9.]*.*$' ''

should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fish's deduplication will only be done if the function is used as input for completion, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. What else do you want to use this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, there is __fish_print_packages, however I didn't quite understand what the purpose of that function is/was.

Copy link
Member

@faho faho Mar 7, 2018

Choose a reason for hiding this comment

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

Completions. It prints packages for a variety of package managers, so you can theoretically have a tool that takes a name of a system package and complete it just by calling that function.

I don't think a tool like that actually exists, and I've wanted to split it up for a while - because right now if you e.g. have pacman installed (e.g. to create an archlinux container) and try to complete yum it'll suggest pacman's package names, because that's first in __fish_print_packages.

@@ -0,0 +1,4 @@
function __fish_portage_print_repository_names --description 'Print the names of all configured repositories'
# repos.conf may be a file or a directory
find /etc/portage/repos.conf -type f -exec sed -ne '/^[[:blank:]]*\[[[:alnum:]_][[:alnum:]_-]*\]/{s!^.*\[\(.*\)\].*$!\1!; /DEFAULT/!p}' '{}' +
Copy link
Member

Choose a reason for hiding this comment

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

What does the input here look like?

Copy link
Member

Choose a reason for hiding this comment

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

And what are you trying to extract?

(This goes for print_repository_paths as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The format is ini-like, as in

[repository]
key = value
# comment

In this case the part between [ ] is extracted, and the special "repository" DEFAULT removed from the output.
In the case of print_repository_paths the value of the key "location" is extracted.

Copy link
Member

Choose a reason for hiding this comment

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

Leave it like this for now. If the completions turn out to be slow, this is my guess. But I'd assume few people have hundreds of repository files configured.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be <5 for most people. Additionally, the files themselves are rather small.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also should completion be slow I'd assume it's because gentoo's main repository alone contains ~20k packages.

complete -c equery -s V -l version -d "display version information"

## Subcommands
complete -c equery -n '__fish_use_subcommand' -xa 'belongs' -d '"'(_ 'list all packages owning file(s)')'"'
Copy link
Member

Choose a reason for hiding this comment

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

Descriptions are translated by default - no need to invoke _ at all.

Just -d 'List all packages owning files'.

Also:

  • Capitalize the first word

  • Keep descriptions short (i.e. no file(s), just files)

Copy link
Contributor Author

@drwilly drwilly Mar 6, 2018

Choose a reason for hiding this comment

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

Good, that part was inherited and I left it in just in case.
Capitalization is done.
Descriptions are copy-pasted from the man-page.


# which
complete -c equery -n '__fish_seen_subcommand_from which' -s m -l include-masked -d "return highest version ebuild available"
Copy link
Member

Choose a reason for hiding this comment

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

Is that actually the definition?

Can't I have a masked ebuild with a lower version number? From my combined four hours of using Gentoo, I recall seeing ebuilds with a version number of "9999" or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The definition is copypasted from equery which -h.
The man-page is a only little more elaborate: "Return the path to the hightest version ebuild available."

complete -c emerge -s l -l changelog -d "show changelog of package. Use with --pretend"
complete -c emerge -l color -d "colorized output" \
-xa "y n"
complete -c emerge -l colums -d "Align output. Use with --pretend"
Copy link
Member

Choose a reason for hiding this comment

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

I think this is "columns".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

#########################

complete -c emerge -s A -l alert -d "add a terminal bell character ('\a') to all interactive prompts"
Copy link
Member

Choose a reason for hiding this comment

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

Does the "interactive" add anything here? Or would just Add a bell ('\a') to all prompts suffice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, -A without -a (or any other flag that causes interaction) has no effect.


## Local opts
# cleanlogs
complete -c emaint -n '__fish_seen_subcommand_from cleanlogs' -s t -l time -d "Delete logs older than NUM of days" \
Copy link
Member

Choose a reason for hiding this comment

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

We sometimes just generate a list of plausible numbers with seq - -xa "(seq 0 60)".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok … I guess.

complete -c emaint -n '__fish_use_subcommand' -xa 'binhost' -d 'Scan and generate metadata indexes for binary packages.'
complete -c emaint -n '__fish_use_subcommand' -xa 'cleanconfmem' -d 'Check and clean the config tracker list for uninstalled packages.'
complete -c emaint -n '__fish_use_subcommand' -xa 'cleanresume' -d 'Discard emerge --resume merge lists'
complete -c emaint -n '__fish_use_subcommand' -xa 'logs' -d 'Check and clean old logs in the PORT_LOGDIR.'
Copy link
Member

Choose a reason for hiding this comment

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

No trailing "." in the descriptions please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

complete -c emaint -s c -l check -d "Check for problems (a default option for most modules)"
complete -c emaint -l version -d "show program's version number and exit"
complete -c emaint -s f -l fix -d "Attempt to fix problems (a default option for most modules)"
complete -c emaint -s P -l purge -d "Removes the list of previously failed merges. WARNING: Only use this option if you plan on manually fixing them or do not want them re-installed."
Copy link
Member

Choose a reason for hiding this comment

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

The warning's a bit long. Maybe just "DANGEROUS"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, copied from emaint's usage output.
The flag isn't all that "dangerous" actually, it's more the help-message that is overly verbose.
Maybe just something along the lines of "see man-page"?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that'll work. Also use the imperative "Remove the list", not "Removes".

@faho
Copy link
Member

faho commented Mar 6, 2018

I'm very sorry for the delay! Reviewed now!

@faho faho added this to the fish-3.0 milestone Mar 6, 2018
Copy link
Member

@faho faho left a comment

Choose a reason for hiding this comment

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

More annoying grammar nitpicks.

@@ -0,0 +1,32 @@
## Global Opts
complete -c emaint -s h -l help -d "show this help message and exit"
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization.

## Global Opts
complete -c emaint -s h -l help -d "show this help message and exit"
complete -c emaint -s c -l check -d "Check for problems (a default option for most modules)"
complete -c emaint -l version -d "show program's version number and exit"
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization.

Also just "Show version and exit" would suffice, I think.

complete -c emaint -s h -l help -d "show this help message and exit"
complete -c emaint -s c -l check -d "Check for problems (a default option for most modules)"
complete -c emaint -l version -d "show program's version number and exit"
complete -c emaint -s f -l fix -d "Attempt to fix problems (a default option for most modules)"
Copy link
Member

Choose a reason for hiding this comment

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

"Attempt to fix problems (default for most modules)"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't ask me, I just copypaste messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

That was a suggestion for improvement.


complete -c emerge -s A -l alert -d "add a terminal bell character ('\a') to all interactive prompts"
#complete -c emerge -l alphabetical -d "Sort flag lists alphabetically"
complete -c emerge -s a -l ask -d "prompt the user before peforming the merge"
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization.

# autounmask-keep-masks
# autounmask-write
complete -c emerge -l backtrack
complete -c emerge -s b -l buildpkg -d "build a binary package additionally"
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization.

complete -c emerge -s u -l update
# use-ebuild-visibility
# useoldpkg-atoms ATOMS
complete -c emerge -s k -l usepkg -d "use binary package if available"
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization.

@drwilly
Copy link
Contributor Author

drwilly commented Mar 13, 2018

I'm not the developer of either of these programs, and all of these messages are from the official documentation.

@faho
Copy link
Member

faho commented Mar 13, 2018

I'm not the developer of either of these programs, and all of these messages are from the official documentation.

Sure. But this isn't the official documentation. This is a completion script for fish.

And these descriptions serve a different purpose than the official documentation. They are here to get you to quickly select the option you want.

That means that they shouldn't be outright wrong, but they should be short and to the point, especially because the shorter they are the more we can use additional columns, which allows for more flexible movement, which means you get the option quicker. (That even means that in some cases they can gloss over some things, but that's not even the case here)

And they should be consistently capitalized because that's just simply less distracting.

@@ -1,32 +1,32 @@
## Global Opts
complete -c emaint -s h -l help -d "show this help message and exit"
complete -c emaint -s c -l check -d "Check for problems (a default option for most modules)"
complete -c emaint -s c -l check -d "check for problems"
Copy link
Member

Choose a reason for hiding this comment

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

Ummmhh.. other way around, please!

It should be "Check for problems", not "check for problems".

@faho faho merged commit 44e2c28 into fish-shell:master Mar 23, 2018
@faho
Copy link
Member

faho commented Mar 23, 2018

Merged, thanks and sorry for the delay!

@drwilly
Copy link
Contributor Author

drwilly commented Mar 24, 2018

No worries.

amosbird pushed a commit to amosbird/fish-shell that referenced this pull request Mar 26, 2018
* revisit gentoo-related completions

- add completion for emaint
- improve completion for emerge
  still incomplete, but an impovement regardless
- improve completion for equery
- add gentoo/portage-related auxiliary functions

* fix spelling

* remove trailing '.'

* remove old '_' invocation and capitalize descriptions

* add number-completion

* remove trailing '.'

* shorter descriptions

* replace sed with fish-builtin and drop deduplication

* batch change capitalization (lower case)

* indent equery descriptions

* batch change capitalization (upper case)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants