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

Unexpected behaviour with quote_representation = '"' #252

Open
sid-maddy opened this issue Jul 23, 2023 · 2 comments
Open

Unexpected behaviour with quote_representation = '"' #252

sid-maddy opened this issue Jul 23, 2023 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@sid-maddy
Copy link

Description

Even with quote_representation = '"' set in the config file, yamlfix quotes already double-quoted values with single quotes.

Steps to reproduce

.yamlfix.toml

quote_representation = '"'

file_with_double_quoted_integer_value.yaml

port: "80"
$ yamlfix --config-file .yamlfix.toml file_with_double_quoted_integer_value.yaml

Current behavior

yamlfix quotes the port value with single quotes.

file_with_double_quoted_integer_value.yaml

---
port: '80'

FWIW, with quote_basic_values = true also set in the config file, it exhibits the desired behaviour.

---
port: "80"

Desired behaviour

yamlfix keeps the port value quoted with double quotes (even without setting quote_basic_values = true.)

Environment

$ yamlfix --version
------------------------------------------------------------------
     yamlfix: 1.12.0
     Python: 3.11.4
     Platform: macOS-13.4.1-arm64-arm-64bit
------------------------------------------------------------------
@sid-maddy sid-maddy added the bug Something isn't working label Jul 23, 2023
@lyz-code
Copy link
Owner

lyz-code commented Aug 1, 2023

Hi @sid-maddy thanks for taking the time to open an issue, I reproduced the bug myself but after a quick look I haven't found a quick way to fix this issue as ruyaml doesn't allow us to set which character to use when quoting.

You can try with the next test (to be added to tests/unit/test_adapter_yaml.py):

    @pytest.mark.parametrize("quote_representation", quote_representations)
    def test_quote_values_without_quote_basic_values(self, quote_representation: str) -> None:
        """Quote only scalar values with configurable quote representation."""
        source = dedent(
            """\
            port: '80'
            """
        )
        quote = quote_representation
        fixed_source = dedent(
            f"""\
            ---
            port: {quote}80{quote}
            """
        )
        config = YamlfixConfig()
        config.quote_representation = quote_representation

        result = fix_code(source, config)

        assert result == fixed_source

And inspire yourself on _configure_quotation_for_basic_values at src/yamlfix/adapters

@lyz-code lyz-code added the help wanted Extra attention is needed label Aug 1, 2023
@jasonwbarnett
Copy link

jasonwbarnett commented Dec 19, 2023

@lyz-code maybe remove the config until the feature is fixed? I just ran into this today and it would have saved me time if the option simply wasn't available and/or it was documented that this is a known gap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants