Endilll wrote: I want to see consistent application of `str.format` across our bindings, with named placeholder where they are beneficial, but without `!r` or other such modifiers. We can revisit this in the future if a compelling case arises. See below for more details.
`CompletionString.__repr__` on line 3190 should use named placeholders to make it clear for readers what list comprehension produces. `CompletionChunk.__repr__` on line 3059 should use regular curly brace escape for format strings. Format string there is rather short, so I don't think workarounds we previously discussed would be justified. --- Theoretically we have four options here: 1. Status quo. 2. Consistently use f-strings. 3. Consistently use `str.format`. 4. Land somewhere in-between options 2 and 3. I think everyone here agrees that we should do something, so option 1 is out. Even in this patch I see how usage of f-strings degrades readability, so I'm not going to agree to option 2. Option 3 is works everywhere, and as a bonus, named placeholders would improve `CompletionString.__repr__` case on line 3190. Option 4 might sound like a good compromise, but it requires formulating a guideline to even have a hope of surviving in the long run. Even if we come up with such guideline, it will have to be taught to both contributors and (future) maintainers, otherwise I can easily see a PR coming up in the future "fixing" existing usages of `str.format` to use "more modern" f-strings, maybe even as an NFC, lowering the guard down. Instead, I'd like people to be able to follow what code around does, which requires consistency that option 3 provides. https://github.com/llvm/llvm-project/pull/173861 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
