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

Feature request > Fix quoted-strings #195

Open
giggio opened this issue Oct 6, 2022 · 8 comments
Open

Feature request > Fix quoted-strings #195

giggio opened this issue Oct 6, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers RUN

Comments

@giggio
Copy link

giggio commented Oct 6, 2022

❔ Context

Fix quoted-strings. Docs:
https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.quoted_strings

💡 Proposed solution

  1. Change quote-type to double or single accordingly;
  2. Remove unnecessary quotes when required is set to only-when-needed;
  3. Add quotes when required is set to true;
  4. Remove quoted quotes when allow-quoted-quotes is false.
@giggio giggio added the enhancement New feature or request label Oct 6, 2022
@adriens adriens added good first issue Good for newcomers RUN labels Oct 6, 2022
@adriens
Copy link
Member

adriens commented Oct 6, 2022

Hi @tamere-allo-peter , what's your opinion about this feature request ?

@tamere-allo-peter
Copy link
Member

@adriens very complex. Currently we don't use yamllint's configuration file, we only pass it as-is to yamllint when needed, and we "fix" by doing what yamllint suggests, whenever it does suggest something, and do our best the remaining of the time. Closing this one is very hard work because if we begin to parse yamllint's configuration file, this opens the door to a number of other fixes we should do according to this file that we don't right now.

@adriens
Copy link
Member

adriens commented Oct 6, 2022

So should we flag this as a wontfix because of lack of time ?

@tamere-allo-peter
Copy link
Member

@adriens we'll leave it open for now, maybe I'll have more time for this at the end of the year.

@adriens
Copy link
Member

adriens commented Oct 6, 2022

So it will stay on :
image

⏸️

@fortin-alex
Copy link

fortin-alex commented Dec 19, 2022

this opens the door to a number of other fixes we should do according to this file that we don't right now.

Perhaps that it opens the door to different fixes, but the scope of the current github issue would remain "to automatically fix quoted-strings" and so the other fixes that could be made can be put in a backlog for future contributions.

This said, another issue would need to be created & resolved first (i.e. Adding a yamllint configuration parser)

ps: thanks for this package, it's really great!

@fortin-alex
Copy link

fortin-alex commented Dec 19, 2022

Re-reading my previous comment, it might be something that yamllint can fix on their end (i.e. they could write a better message when reporting a quoted-strings error)

On my end, I see this message from yamllint:

>>> cat .yamllint
rules:
 quoted-strings:
    quote-type: double
    required: true
    allow-quoted-quotes: true

>>> yamllint --format parsable .
./path/to/file/config.yaml:31:11: [error] string value is not quoted with double quotes (quoted-strings)

And I am wondering if this message is informative enough to be actionable? it seems to suggest to replace the single quotes by double quotes or to add double-quotes.

@tamere-allo-peter
Copy link
Member

Yes it seems to be informative enough in the general case, however unfortunately I don't have the time to work on this project at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers RUN
Projects
None yet
Development

No branches or pull requests

4 participants