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

Parse quoted strings for ckan prompt #3889

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 22, 2023

Problems

Cause

That module's version has a space in it, which ends up in the .ckan filename. This can already be handled from the raw command line by using your shell's quoting abilities, but the metadata validator does its work in a ckan prompt session, which doesn't do quoting.

https://github.com/KSP-CKAN/xKAN-meta_testing/blob/master/ckan_meta_tester/ckan_install_template.txt

install -c $ckanfile --headless

After parsing, the code to execute the install command just sees the filename up to the first space.

Changes

  • Now if you put double quotes around a string in ckan prompt, it will not be split on spaces, which will allow us to solve the original problem in a future pull request to xKAN-meta_testing by adding spaces around the filename
  • Now the install --ckanfiles option can be tab-completed
  • Now the common -- options like --instance are included in tab-completion rather than just the verb-specific ones
  • Now single-dash options like -c can be tab-completed as well
  • Trivial code style updates in Meta.cs
  • Meta.cs.in is deleted. This was added in Use Cake for build and overhaul/cleanup build #1589, I think as an intermediate stage of those changes in which version info would be populated into Meta.cs at compile time by the build system, but subsequently made unnecessary by retrieving those changes from the assembly attributes instead. As far as I can tell from examining the commit log, this file has never actually been used.

@HebaruSan HebaruSan added Enhancement New features or functionality Cmdline Issues affecting the command line labels Aug 22, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

All makes sense, thanks @HebaruSan !

I note the scanner has a bunch of notices about unreachable code? Benign or just related specifically .net core?

@HebaruSan

This comment was marked as resolved.

@techman83
Copy link
Member

Ahh, neither—those are something I should fix...

Awesome, if there are no major changes involved with them I'm happy enough with my review as is. So feel free to merge when you're sorted with them.

@HebaruSan

This comment was marked as resolved.

@HebaruSan HebaruSan merged commit b916779 into KSP-CKAN:master Aug 23, 2023
10 checks passed
@HebaruSan HebaruSan deleted the fix/cmdline-prompt-quoting branch August 23, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line Enhancement New features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants