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

[Tracking] Decide what to do with Period 2 spaces in quoted strings and Resource Strings #12033

Closed
paul1956 opened this issue Sep 2, 2024 · 7 comments

Comments

@paul1956
Copy link
Contributor

paul1956 commented Sep 2, 2024

Issue #11984 lists replacing a period followed by 2 spaces as an issue to be fixed but they are also used in quoted string and translated resource strings and changing them there could be breaking or at least will require translation work, so it is separate from PR #12028.

@Tanya-Solyanik
Copy link
Member

75 hits in the resx files. Sample <value>Unable to obtain a stream for the log. Potential file names based on {0} are already in use.</value>

@Tanya-Solyanik Tanya-Solyanik added the untriaged The team needs to look at this issue in the next triage label Sep 3, 2024
@paul1956
Copy link
Contributor Author

paul1956 commented Sep 3, 2024

@Tanya-Solyanik I am afraid to change them without knowing how they might affect tests and customer code, plus how to get the new string translated. Same issue with quoted strings. If the team wants them changed, I have the time and interest in doing it.

If there is a different decision for quoted strings and resx files this issue should be split.

@Tanya-Solyanik
Copy link
Member

@paul1956 - I was just trying to understand what kind of changes these are. As I see it, these strings are mostly exception messages, and thus user experience will not be drastically improved by such a change. Only 1 string is an actual UI text that could be changed. We'll triage it, but my vote is to won't fix this issue.

@paul1956
Copy link
Contributor Author

paul1956 commented Sep 3, 2024

@Tanya-Solyanik I think there are 73-75 in Resx files with period space space, that could be changed (I have no opinion because it's additional work and tests, and I don't know how to properly edit Resx files). There are still 34 in .cs files, a few somehow got missed, most are in Debug Asserts (missed intentionally) and the rest in inline comments in code and not Document Comments. I would fix the comments and as you advise move those comments onto previous line as its hard to even see them, I did that for VB already.

There are also 223 places with multiple spaces between words. at least some in Code that VS should have removed automatically.

I any case its 1 or more new PR's. Please advise.

@Tanya-Solyanik
Copy link
Member

We will triage resx files. I personally don't see a benefit in changing the exception messages.

Smaller PRs dedicated to a single REGEX run are the easiest to review.

That could be a single PR:

There are also 223 places with multiple spaces between words.

And moving comments above the field declarations would be another dedicaed PR

@paul1956
Copy link
Contributor Author

paul1956 commented Sep 4, 2024

@Tanya-Solyanik REGEX fixing is very dangerous because Tests have many places with 2 spaces that should not be changed. REGEX is great for finding candidates. As PR is merged, I will tackle another cleanup issue.

  1. Fix of 2 spaces between characters
  2. Long lines
  3. If necessary, move inline comments on property declarations to previous line

Also would line to make progress getting reviews of VB Code PR's

@merriemcgaw
Copy link
Member

Triage consensus is that there's not enough benefit for the re-localization cost that we'd be incurring. Thanks @paul1956 for the issue - it spawned a great discussion!

@dotnet-policy-service dotnet-policy-service bot removed the untriaged The team needs to look at this issue in the next triage label Sep 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants