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

Add fish shell completion script #58

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

playonverbs
Copy link

As mentioned by @hedythedev in #2 this adds a completion script for the fish shell. The category file location is read from $HOME/config/, else it defaults to $HOME/stup.

One small issue is the short completion -@, which provides autocompletes for -@n, -@c, -@h and -@v instead.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I don’t know how the completion system works so I’ll check that later

#!/usr/bin/env fish

set -l times week month year previous-week previous-month previous-year
set -l commands show add edit copy log search add-category list-category set-category order-category rename-category usage
Copy link

Choose a reason for hiding this comment

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

list-categor*ies*, set-category*-description*, order-categor*ies*

& version?

Copy link
Author

Choose a reason for hiding this comment

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

I have had an issue with the stup version command returning Error: Unknown command "version" (using both fish and bash). This is why I originally didn't include the subcommand.

However it's functionality that should work and the error can be raised in a separate issue, so I'll add the subcommand.

Copy link

Choose a reason for hiding this comment

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

Ah, you’re right. I didn’t check if it works.

completions/stup_complete.fish Outdated Show resolved Hide resolved
set -l desc_cat_commands add-category set-category

# read category file path from config. default to ~/stup
cat "$HOME/.config/stup.conf" 2> /dev/null | read -d = -l _ category_dir ; or set -l category_dir "$HOME/stup"
Copy link

Choose a reason for hiding this comment

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

$XDG_CONFIG_HOME pls

stup/stup

Line 1119 in da6d23d

CONFIG_FILE="${XDG_CONFIG_HOME:-$HOME/.config}/stup.conf"

Copy link
Author

Choose a reason for hiding this comment

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

$HOME/.config has been replaced with $XDG_CONFIG_HOME

Copy link

Choose a reason for hiding this comment

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

Then comes the issue when $XDG_CONFIG_HOME is not set…

Does fish not have that simple fallback system?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Sorry, I’ve been a little busy, but finally I’ve gotten to look at it in more detail.

  • regarding -@ completing other short options, that seems to be a problem of fish (& is the same for all short options). I really don’t understand how it can complete a row of short options (-abcdef) & at the same time only complete predefined arguments tightly attached to the short option (-ccategory). That’s a nonsense at the very least. (I hate hate hate fish for things like this)
    • so probably you can’t do anything with this
  • again fish — only completes arguments to long options with =, which is (like with short options) not supported by stup (yet)
    • again, there’s probably nothing you can do with this
  • why do you add description only to some options?

I won’t mess into this more (hopefully) & leave it to the owner/author/maintainer, @iridakos

set -l desc_cat_commands add-category set-category-description

# read category file path from config. default to ~/stup
cat "$XDG_CONFIG_HOME/stup.conf" 2> /dev/null | read -d = -l _ category_dir ; or set -l category_dir "$HOME/stup"
Copy link

Choose a reason for hiding this comment

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

Okay, I won’t require ${XDG_CONFIG_HOME:-$HOME/.config} (unless @iridakos does) since I’ve realised how fish makes this more verbose than other shells

complete -c stup -n "__fish_seen_subcommand_from $dated_commands" -s "@" -l at -d "Note timestamp"

# from/to options
complete -c stup -n "__fish_seen_subcommand_from $ranged_commands" -s f -l from
Copy link

Choose a reason for hiding this comment

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

There are no -f & -t, the same in the following lines

complete -c stup -n "__fish_seen_subcommand_from $ranged_commands" -s f -l from
complete -c stup -n "__fish_seen_subcommand_from $ranged_commands" -s t -l to

complete -c stup -n "__fish_contains_opt -s @ at ; and __fish_seen_subcommand_from $dated_commands" -x -a "$times"
Copy link

Choose a reason for hiding this comment

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

This should complete dates, not ranges

# subcommands listed in $commands
complete -c stup -n "not __fish_seen_subcommand_from $commands" -x -a "$commands"

# default `add` behaviour with no subcommands
Copy link

Choose a reason for hiding this comment

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

The default command can be changed in the config file, but respecting it could awfully increase complexity… (so I don’t require it)


# add (no-)pager option for paged commands
complete -c stup -n "__fish_seen_subcommand_from $paged_commands" -l no-pager -d "Disable pager"
complete -c stup -n "__fish_seen_subcommand_from $paged_commands" -l pager -d "Use pager"
Copy link

Choose a reason for hiding this comment

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

--pager requires an argument (program to execute or false)

@ghost
Copy link

ghost commented Jan 26, 2021

Oh, still there is something that I forgot: installation instructions — not needed, but would be good.

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.

1 participant