Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_cli): remove unsupported options in the ci help text #3457

Merged
merged 2 commits into from
Oct 20, 2022
Merged

fix(rome_cli): remove unsupported options in the ci help text #3457

merged 2 commits into from
Oct 20, 2022

Conversation

lucasweng
Copy link
Contributor

Summary

  • Resolves 🐛 rome ci --help prints unsupported options #3456

  • Per apply_format_settings_from_cli

    let configuration = apply_format_settings_from_cli(&mut session, configuration)?;

    rome ci supports the following options:

      --indent-style <tabs|space>              Change the indention character (default: tabs)
      --indent-size <number>                   If the indentation style is set to spaces, determine how many spaces should be used for indentation (default: 2)
      --line-width <number>                    Change how many characters the formatter is allowed to print in a single line (default: 80)
      --quote-style <single|double>            Changes the quotation character for strings (default: ")
      --quote-properties <as-needed|preserve>  Changes when properties in object should be quoted (default: as-needed)
      --trailing-comma <all|es5>               Changes trailing commas in multi-line comma-separated syntactic structures (default: all)
    

Test Plan

  • Running cargo run --bin rome ci --help should print out the supported options
    options

@lucasweng lucasweng requested a review from leops as a code owner October 19, 2022 15:57
@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit e5ab712
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6351480442ecec0008432248
😎 Deploy Preview https://deploy-preview-3457--docs-rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@MichaReiser MichaReiser added the hacktoberfest Issues for hacktoberfest label Oct 19, 2022
@MichaReiser
Copy link
Contributor

Good catch that the rome format options are supported.

Does this change also fix the issue that rome ci incorrectly refers to the rome format command when an argument is missing?

❯ cargo run --bin rome ci --quote-style="double" 
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/rome ci --quote-style=double`
Error: missing argument '<INPUT>'. Type 'rome format --help' for more information.

@lucasweng
Copy link
Contributor Author

lucasweng commented Oct 19, 2022

Good catch that the rome format options are supported.

Does this change also fix the issue that rome ci incorrectly refers to the rome format command when an argument is missing?

❯ cargo run --bin rome ci --quote-style="double" 
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/rome ci --quote-style=double`
Error: missing argument '<INPUT>'. Type 'rome format --help' for more information.

Didn't catch that.. Thanks for pointing that out! Will fix the issue here

Edit:

@lucasweng

This comment was marked as resolved.

Comment on lines 63 to 70
"<Emphasis>"OPTIONS:"</Emphasis>"
"<Dim>"--indent-style <tabs|space>"</Dim>" Change the indention character (default: tabs)
"<Dim>"--indent-size <number>"</Dim>" If the indentation style is set to spaces, determine how many spaces should be used for indentation (default: 2)
"<Dim>"--line-width <number>"</Dim>" Change how many characters the formatter is allowed to print in a single line (default: 80)
"<Dim>"--quote-style <single|double>"</Dim>" Changes the quotation character for strings (default: \")
"<Dim>"--quote-properties <as-needed|preserve>"</Dim>" Changes when properties in object should be quoted (default: as-needed)
"<Dim>"--trailing-comma <all|es5>"</Dim>" Changes trailing commas in multi-line comma-separated syntactic structures (default: all)
"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer it if we change FORMAT_OPTIONS to only contain the markup for the format options (without the OPTIONS header and then compose it in format and rome ci

const FORMAT_OPTIONS: Markup = markup! {
	"<Dim>"--indent-style <tabs|space>"</Dim>"              Change the indention character (default: tabs)
	"<Dim>"--indent-size <number>"</Dim>"                   If the indentation style is set to spaces, determine how many spaces should be used for indentation (default: 2)
	"<Dim>"--line-width <number>"</Dim>"                    Change how many characters the formatter is allowed to print in a single line (default: 80)
	"<Dim>"--quote-style <single|double>"</Dim>"            Changes the quotation character for strings (default: \")
	"<Dim>"--quote-properties <as-needed|preserve>"</Dim>"  Changes when properties in object should be quoted (default: as-needed)
	"<Dim>"--trailing-comma <all|es5>"</Dim>"               Changes trailing commas in multi-line comma-separated syntactic structures (default: all)
}
Suggested change
"<Emphasis>"OPTIONS:"</Emphasis>"
"<Dim>"--indent-style <tabs|space>"</Dim>" Change the indention character (default: tabs)
"<Dim>"--indent-size <number>"</Dim>" If the indentation style is set to spaces, determine how many spaces should be used for indentation (default: 2)
"<Dim>"--line-width <number>"</Dim>" Change how many characters the formatter is allowed to print in a single line (default: 80)
"<Dim>"--quote-style <single|double>"</Dim>" Changes the quotation character for strings (default: \")
"<Dim>"--quote-properties <as-needed|preserve>"</Dim>" Changes when properties in object should be quoted (default: as-needed)
"<Dim>"--trailing-comma <all|es5>"</Dim>" Changes trailing commas in multi-line comma-separated syntactic structures (default: all)
"
"<Emphasis>"OPTIONS:"</Emphasis>"
{FORMAT_OPTIONS}
"

and for rome_format

... 
"<Emphasis>"OPTIONS:"</Emphasis>""
	"<Dim>"--write"</Dim>"                                  Edit the files in place (beware!) instead of printing the diff to the console
	"<Dim>"--skip-errors"</Dim>" Skip over files containing syntax errors instead of emitting an error diagnostic.
    {FORMAT_OPTIONS}
	"<Dim>"--stdin-file-path <string>"</Dim>" A file name with its extension to pass when reading from standard in, e.g. echo 'let a;' | rome format --stdin-file-path file.js
};

This allows us to keep the format option documentation centralized.

Copy link
Contributor Author

@lucasweng lucasweng Oct 20, 2022

Choose a reason for hiding this comment

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

e5ab712 I tested the formatting of the help text
rome format
rome ci

@MichaReiser
Copy link
Contributor

MichaReiser commented Oct 20, 2022

Brilliant. Thank you!

@MichaReiser MichaReiser merged commit 4f22f17 into rome:main Oct 20, 2022
@lucasweng lucasweng deleted the fix/ci-help-text branch October 21, 2022 01:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
hacktoberfest Issues for hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 rome ci --help prints unsupported options
3 participants