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

New completions: figlet, mdbook, ranger, pygmentize + xz fix #3378

Closed
wants to merge 9 commits into from
Closed

New completions: figlet, mdbook, ranger, pygmentize + xz fix #3378

wants to merge 9 commits into from

Conversation

moverest
Copy link
Contributor

Add figlet, mdbook, ranger and pygmentize and fix xz completions.

The previous completion gave every files.
Here, we only show files with the .xz, .txz, .lzma or .tlz extension.
return 1
end

complete -n "__test_args" -d Hello
Copy link
Member

Choose a reason for hiding this comment

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

I believe you didn't want to include this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed...

@floam
Copy link
Member

floam commented Sep 14, 2016

Totally unrelated, but this thread made me notice that OS X comes with pygments, and pygmentize can highlight fish script by default. Neat.

Nope - something installed it into /usr/bin other than Apple, or pkgutil would show a pkgid here. Nevermind!

pkgutil --file-info /usr/bin/pygmentize
volume: /
path: /usr/bin/pygmentize

@moverest
Copy link
Contributor Author

I really like this tool. I have defined an alias function cats which does pygmentize -g. It works quite well with fish scripts.

@floam
Copy link
Member

floam commented Sep 14, 2016

fish_indent --ansi highlights better :) But it only does fish.

@moverest
Copy link
Contributor Author

That's why I use pygmentize. I can throw everything at it without having to think much and the highlights are pretty good. It doesn't even need the extension to guess some languages.

@floam floam changed the title Extend completion support New completions: figlet, mdbook, ranger, pygmentize + xz fix Sep 15, 2016
@@ -0,0 +1,29 @@
function __fish_print_figlet_fonts
find (figlet -I2) -type f ^ /dev/null | sed -e 's$/.*/\([^/.]*\)\.[ft]lf$\1\tFont$;tx;d;:x'
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 figlet -I2 output?

This might be nicer with string, though it's something we can also do later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

> figlet -I2
/usr/share/figlet/fonts
> find (figlet -I2) -type -f ^ /dev/null
/usr/share/figlet/fonts/slant.flf
/usr/share/figlet/fonts/ilhebrew.flc
/usr/share/figlet/fonts/banner.flf
/usr/share/figlet/fonts/646-jp.flc
/usr/share/figlet/fonts/646-irv.flc
/usr/share/figlet/fonts/646-es.flc
/usr/share/figlet/fonts/8859-4.flc
/usr/share/figlet/fonts/646-cn.flc
/usr/share/figlet/fonts/646-fr.flc
/usr/share/figlet/fonts/standard.flf
/usr/share/figlet/fonts/646-pt.flc
/usr/share/figlet/fonts/ivrit.flf
/usr/share/figlet/fonts/hz.flc
... and some more ...

We only want the name of the fonts (files with the extension .flf or .tlf). Here, that would be:

slant
banner
standard
... and some more ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does the same thing:

function __fish_print_figlet_fonts
    set -l lines (find (figlet -I 2) -type f | string match -r '/.*/(.*)\.[ft]lc')

    while set -q lines[2]
        printf '%s\tFont\n' $lines[2]
        set -e lines[1]
        set -e lines[1]
    end
end

Copy link
Member

Choose a reason for hiding this comment

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

How about

set -l dir (figlet -I 2)
set -l files $dir/*.flf $dir/*.tlf

printf '%s\tFont\n' (string replace -r '.*/([^/]+)\.[ft]lf' '$1' -- $files)

?

You might want to test the performance here. To do that, define the function, execute it and echo $CMD_DURATION right after. That'll give you the time in milliseconds.

Copy link
Contributor Author

@moverest moverest Sep 16, 2016

Choose a reason for hiding this comment

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

Your solution is much faster (yours: 4ms; sed: 12ms; find + match: 25ms). I'll commit it.

Copy link
Member

Choose a reason for hiding this comment

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

So my intuition was correct. The trick, and this applies to all shells, is to use builtins unless you're dealing with large datasets, when specialized tools (GNU's grep is blindlingly fast) might make sense. There's a constant cost to forking off an external command.

@faho
Copy link
Member

faho commented Sep 16, 2016

Merged as ed7bf83..fc3cd77 (I squashed the improvements on top of the original commit).

Thanks!

@faho faho closed this Sep 16, 2016
@moverest
Copy link
Contributor Author

moverest commented Sep 16, 2016

I have to say that I really like to contribute here. I am always learning something new.
I may have some more in the near future (feh, aria2c).

@zanchey
Copy link
Member

zanchey commented Sep 18, 2016

These commits have been lost somehow from master.

@zanchey zanchey reopened this Sep 18, 2016
@zanchey
Copy link
Member

zanchey commented Sep 18, 2016

Fixed with 92e3a3c - sorry for the noise.

@zanchey zanchey closed this Sep 18, 2016
@moverest moverest deleted the completions branch September 22, 2016 08:50
@moverest moverest restored the completions branch September 22, 2016 21:19
@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

4 participants