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

Conversation

Projects
None yet
2 participants
@drwilly
Copy link
Contributor

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
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
@@ -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

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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.

This comment has been minimized.

@drwilly

drwilly Mar 7, 2018

Contributor

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

This comment has been minimized.

@faho

faho Mar 7, 2018

Member

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

This comment has been minimized.

@drwilly

drwilly Mar 7, 2018

Contributor

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

This comment has been minimized.

@faho

faho Mar 7, 2018

Member

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}' '{}' +

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

What does the input here look like?

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

And what are you trying to extract?

(This goes for print_repository_paths as well)

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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.

This comment has been minimized.

@faho

faho Mar 7, 2018

Member

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.

This comment has been minimized.

@drwilly

drwilly Mar 7, 2018

Contributor

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

This comment has been minimized.

@drwilly

drwilly Mar 7, 2018

Contributor

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)')'"'

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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)

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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"

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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.

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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"

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

I think this is "columns".

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

Yes

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

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

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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" \

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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

This comment has been minimized.

@drwilly

drwilly Mar 7, 2018

Contributor

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.'

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

No trailing "." in the descriptions please.

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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."

This comment has been minimized.

@faho

faho Mar 6, 2018

Member

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

This comment has been minimized.

@drwilly

drwilly Mar 6, 2018

Contributor

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"?

This comment has been minimized.

@faho

faho Mar 7, 2018

Member

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

@faho

This comment has been minimized.

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

@faho
Copy link
Member

faho left a comment

More annoying grammar nitpicks.

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

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

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"

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

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)"

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

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

This comment has been minimized.

@drwilly

drwilly Mar 13, 2018

Contributor

Don't ask me, I just copypaste messages.

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

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"

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

Capitalization.

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

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

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"

This comment has been minimized.

@faho

faho Mar 13, 2018

Member

Capitalization.

@drwilly

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

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"

This comment has been minimized.

@faho

faho Mar 14, 2018

Member

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faho

This comment has been minimized.

Copy link
Member

faho commented Mar 23, 2018

Merged, thanks and sorry for the delay!

@drwilly

This comment has been minimized.

Copy link
Contributor

drwilly commented Mar 24, 2018

No worries.

amosbird added a commit to amosbird/fish-shell that referenced this pull request Mar 26, 2018

revisit gentoo-related completions (fish-shell#4758)
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment