Skip to content

Upgrade Mono completions#8452

Closed
EmilyGraceSeville7cf wants to merge 28 commits into
masterfrom
unknown repository
Closed

Upgrade Mono completions#8452
EmilyGraceSeville7cf wants to merge 28 commits into
masterfrom
unknown repository

Conversation

@EmilyGraceSeville7cf
Copy link
Copy Markdown
Contributor

@EmilyGraceSeville7cf EmilyGraceSeville7cf commented Nov 17, 2021

Description

This PR upgrades mono.fish completions and adds completions for other Mono tools.

image

TODOs:

  • mono support
  • gacutil support
  • xsp support
  • mkbundle support
  • monodis support
  • ikdasm support
  • sqlsharp support
  • Gendarme support

@EmilyGraceSeville7cf
Copy link
Copy Markdown
Contributor Author

EmilyGraceSeville7cf commented Nov 19, 2021

I think I have to discuss (and maybe fix) raised problems with option parsing with Mono developers because they aren't not Fish shell problems.

@EmilyGraceSeville7cf
Copy link
Copy Markdown
Contributor Author

Until Mono developers fix mono/mono#21318 I can't write normal completions for some commands:

  • mcs
  • ilasm
  • monop
  • xsd
  • wsdl & wsdl2
  • disco
  • soapsuds

But of course basic completions will be created for them anyway (but in another PR later).

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft November 26, 2021 12:55
@EmilyGraceSeville7cf
Copy link
Copy Markdown
Contributor Author

I'll check these completions tomorrow and refresh them to better fit Fish code style.

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as ready for review January 25, 2022 10:56
Comment thread share/completions/mono.fish Outdated
Comment thread share/completions/mono.fish Outdated
Comment thread share/completions/mono.fish Outdated
complete -c mono -l ncompile \
-d 'Instruct the runtime on the number of times that the method(-s) specified by --compile/--compile-all to be compiled'
complete -c mono -l stats \
-d 'Display information about the work done by the runtime during the execution of an application'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about putting each argument in a new line, for better readability?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
-d 'Display information about the work done by the runtime during the execution of an application'
complete -c mono \
-l ncompile \
-d 'Instruct the runtime on the number of times that the method(-s) specified by --compile/--compile-all to be compiled'
complete -c mono \
-l stats \
-d 'Display information about the work done by the runtime during the execution of an application'

Do you mean smth like that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry the location of my comment is off (bug in my review script) it's about the --aot option (and other in that style)

So I meant that instead of

complete -c mono -l aot -f -a 'asmonly bind-to-runtime-version data-outfile direct-icalls'

I'd write

complete -c mono -l aot -f -a '
    asmonly
    bind-to-runtime-version
    data-outfile
    direct-icalls
'

though there are arguments for both styles

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please check it now. ;)

Comment thread share/completions/mono.fish Outdated
Comment thread share/completions/gacutil.fish Outdated
Comment thread share/completions/gacutil.fish Outdated
Comment thread share/completions/gacutil.fish Outdated
Comment thread share/completions/mkbundle.fish Outdated
Comment thread share/completions/mono.fish
Comment thread share/completions/gacutil.fish Outdated
Comment thread share/completions/mkbundle.fish
@krobelus
Copy link
Copy Markdown
Contributor

Overall this is really great work!

- prefer short options
- fix types
- simplify quoting
- prefer builtin functions
@krobelus krobelus closed this in 1b12719 Feb 5, 2022
@krobelus
Copy link
Copy Markdown
Contributor

krobelus commented Feb 5, 2022

Merged as 1b12719, thanks!

@krobelus krobelus added this to the fish 3.4.0 milestone Feb 5, 2022
@EmilyGraceSeville7cf EmilyGraceSeville7cf deleted the mono-complete-upgrade branch February 5, 2022 19:12
@IlanCosman IlanCosman mentioned this pull request Feb 18, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 6, 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.

2 participants