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

Allow (formatter?) config to never introduce implicitly concatenated strings or just accept that a check run will complain about it #14638

Open
jankatins opened this issue Nov 27, 2024 · 4 comments
Labels
question Asking for support or clarification

Comments

@jankatins
Copy link
Contributor

jankatins commented Nov 27, 2024

I think implicitly concat'ed strings are pure evil since a time where I had to debug an accidentally added comma in a place where + would have been proper, which made a single string a tuple of two strings. I would like ruff to allow one to configure that "no explicit string concat, and not fix it such a way, ever" and not complain about such a config, but currently it does complain and IMO it complains for the wrong reasons and with the wrong solution:

I don't want it to stop complaining about implicitly concat'ed strings, but stop the formatter from introducing them by using a '+' between multiline strings. Or if the formatter needs to introduce implicitly concat'ed strings (because + would be a black violation), just let ruff check complain (when it happens), so I'm forced to fix it by hand (which I would be fine with). But having the scary warning in all runs makes me and especially unwary users scared into disabling that rule. :-(

Inspired by #13031 (comment), I would have expected that

[tool.ruff.lint]
extend-select = ["ISC"]
ignore=["ISC003"] # Don't complain about explicitly concat'ed strings
unfixable = ["ISC001"] # don't fix one line implicitly concat'ed strings, but just complain and make the user fix it

[tool.ruff.lint.flake8-implicit-str-concat]
allow-multiline = false

would work, but ruff still complains, that this is incompatible with the formatter:

 The following rule may cause conflicts when used with the formatter: `ISC001`. To avoid unexpected behavior, we recommend disabling this rule, either by removing it from the `select` or `extend-select` configuration, or adding it to the `ignore` configuration.
@MichaReiser
Copy link
Member

MichaReiser commented Nov 27, 2024

I think what you're asking for will be addressed with the upcoming Ruff 2025 style guide (#13371). The relevant changes are:

  • The formatter will merge implicitly concatenated strings that fit on a single line into a single string literal (there are a few edge cases where that's not possible but they are very rare)
  • The formatter will never collapse an implicit concatenated string Disallow single-line implicit concatenated strings #13928

These two changes will make the formatter compatible with ISC001 (also see #8272).

@MichaReiser MichaReiser added the question Asking for support or clarification label Nov 27, 2024
@MichaReiser
Copy link
Member

would work, but ruff still complains, that this is incompatible with the formatter:

This might be a bug but I'm not sure if it's still worth fixing considering that the new style guide will be released shortly. But feel free to submit a PR that suppresses the warning if that setting is set.

@AlexWaygood AlexWaygood changed the title Allow (formatter?) config to never introduce implicit concatted strings or just accept that a check run will complain about it Allow (formatter?) config to never introduce implicitly concatenated strings or just accept that a check run will complain about it Nov 27, 2024
@jankatins
Copy link
Contributor Author

jankatins commented Nov 28, 2024

The formatter will merge implicitly concatenated strings that fit on a single line into a single string literal (there are a few edge cases where that's not possible but they are very rare)

This is actually something I don't want: up to now, all such cases of mine were bad coding from my side and in a lot of cases I had to pick a comma instead of a plus. Just merging the strings will mean that ["A", "B" "C"] will silently end up as ["A", "BC"] which usually is a bug in my case.

@MichaReiser
Copy link
Member

I see. So the problem here is that you introduce implicit concatenated strings and you want the formatter to leave them unchanged?

I don't think we should introduce an option for this because it is in direct conflict with consistent formatting. However, I do see how the formatter joining the strings can be "annoying" in an editor context. IMO, the benefit of the formatter joining the strings is that it becomes very apparent that you forgot a , and hitting Ctrl + Z in the editor should undo the formatter change.

but stop the formatter from introducing them by using a '+' between multiline strings. Or if the formatter needs to introduce implicitly concat'ed strings (because + would be a black violation),

The formatter never introduces implicit concatenated strings. It only formats implicit concatenated strings that were already present in the source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

2 participants