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

Windows command completions #8486

Closed
wants to merge 63 commits into from
Closed

Windows command completions #8486

wants to merge 63 commits into from

Conversation

EmilyGraceSeville7cf
Copy link
Contributor

@EmilyGraceSeville7cf EmilyGraceSeville7cf commented Nov 25, 2021

Description

Advanced command completions for 13 Windows commands.

Main features

  • Where option requires smth but path option values are suggested (if exist)
  • Where there are two or more mutually exclusive options the first time one of them is suggested (to choose another user have to remove written option)

ToDos

  • cmdkey
  • reg
  • setx
  • attrib
  • attributes
  • choice
  • clean
  • cleanmgr
  • cmd
  • comp
  • forfiles
  • schtasks
  • powershell
  • __fish_seen_argument clean-up and Windows-style option support
  • make concise descriptions

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft November 25, 2021 11:39
@floam
Copy link
Member

floam commented Dec 1, 2021

(Same goes for VER for example...)

@EmilyGraceSeville7cf
Copy link
Contributor Author

Are you certain cls even does anything on fish on Windows? My understanding is there is no cls.exe - it is an internal cmd.exe builtin. Not in PATH, right?

Yes, it's my mistake I've created completion for it. I will go through this command list and remove completions for internal cmd commands.

@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Dec 2, 2021

I've done the small research on manned.org for utils with the same names and found out:

There is only one "conflicting" completion in zsh-completions - for reg command: _reg.

@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Dec 15, 2021

Everything is ready, please review this PR and accept if it's good enough. ;)

h\tClear the Hidden file attribute
i\tClear the Not Content Indexed file attribute' | awk -F '\t' "{ printf \"$current_token%s\t%s\n\", \$1, \$2 }"
case '*'
if __fish_seen_argument --windows 's'
Copy link
Member

Choose a reason for hiding this comment

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

If I run this, I get __fish_seen_argument: --windows: unknown option - probably this needs to be

Suggested change
if __fish_seen_argument --windows 's'
if __fish_seen_argument -- --windows 's'

Copy link
Contributor Author

@EmilyGraceSeville7cf EmilyGraceSeville7cf Jan 24, 2022

Choose a reason for hiding this comment

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

Everything works nice on my machine. 🤔 I've copied completion to my terminal and tested with a fake empty attrib executable.

image

Maybe I am doing smth wrong which prevents me from reproducing this issue.

/?\tShow help'
end

complete --command cleanmgr --no-files --arguments '(__cleanmgr_generate_args)'
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 can be rewritten to be similar to other completions with __fish_seen_subcommand and similar. @faho might have a better idea.

share/completions/cmd.fish Outdated Show resolved Hide resolved
C\tLight red
D\tLight purple
E\tLight yellow
F\tBright white' | awk -F '\t' "{ printf \"$current_token%s\t%s\n\", \$1, \$2 }"
Copy link
Member

Choose a reason for hiding this comment

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

This awk pipeline is pretty gnarly! I think it could be done with Cartesian expansion instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do smth like this:

echo prefix_(echo "1
2")

it prints prefix_1 prefix_2. The problem is dealing with embedded spaces:

echo prefix_(echo "1
2 after-space")

How can I print each item on a separate line to pass them as a suggested completions?

Copy link
Member

Choose a reason for hiding this comment

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

printf "$current_token" is dangerous because `$current_token might contain a format specifier.

How can I print each item on a separate line to pass them as a suggested completions?

You can use printf %s\n instead of echo.

Here's the full Cartesian product expansion:

printf %s\n $current_token(
            printf %s\t%s\n \
                1 Blue \
                2 Green
        )

share/completions/cmd.fish Outdated Show resolved Hide resolved
@zanchey
Copy link
Member

zanchey commented Dec 27, 2021

Thanks for this massive contribution!

Before merging I think it needs to be brought into line with the way most of the other completions are written. That means shortening descriptions, using more complete statements and delegating less to functions, and making the function names more consistent (generally __fish_print_xyz for functions that output lists, and __fish_complete_xyz for functions that output item-description pairs). I've left a hodgepodge of comments above along these lines, but it applies to the majority of files.

It might be easier to just work on one or two at a time, if you like.

EmilySeville7cfg added 7 commits January 24, 2022 15:36
- new name is __fish_print_windows_drives
- rename *list* functions to *print*
- use short options everywhere
- delegate less work to functions
- attributes
- reg
- schtasks
ridiculousfish added a commit that referenced this pull request Jan 27, 2022
This relnotes completions from #8486
@ridiculousfish
Copy link
Member

Rebased and merged ending at cdae653, relnoted at 30216b2 . Thank you!

@ridiculousfish ridiculousfish added this to the fish 3.4.0 milestone Jan 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 30, 2023
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.

5 participants