Calculator: Update Calc x:Name values to begin with uppercase

Created on 27 Feb 2019  Â·  16Comments  Â·  Source: microsoft/calculator

Any and all occurrences of x:Name should use Pascal-case. Right now, it is inconsistent between views. The reasoning is that x:Name converts the object to be accessible as a property of the current view and properties should be Pascal-cased. I think this issue is easily fixed with a regex + replace.

3 codebase quality help wanted

Most helpful comment

For scenarios like this, where there's an unwritten rule about naming conventions, I like to create a test (or tests) to ensure any future changes follow the rule too.
How do others feel about this?
Such a test would iterate over all XAML files in the project and fail if any of the defined u:Id or x:Name values do not have the correct casing pattern.
Would such a test be appropriate in the 'CalculatorUnitTests' project or should it go somewhere else?

All 16 comments

This is your friendly Microsoft Issue Bot. I created this issue automatically as requested by a team member.

Can I get clarification on which views are to be updated?

My understanding of the request is the x:Name and x:Uid of XAML-defined UIElement should use Pascal casing:

Ex.

<TextBlock x:Name="firstNameTextBox" />

Should be

<TextBlock x:Name="FirstNameTextBox" />

Any and all occurrences of x:Name should use Pascal-case. Right now, it is inconsistent between views. The reasoning is that x:Name converts the object to be accessible as a property of the current view and properties should be Pascal-cased. I think this issue is easily fixed with a regex + replace.

x:Uid I am less concerned with, although following the same convention feels appropriate. This would require updating the values in the resw files. Also accomplishable with Replace.

Okay, I can take this one

PS - I'm avoiding modifying C++ as much as possible if you couldn't tell already :)

That's fine, there is a lot of impactful work that can be done in just XAML. I do enjoy discussing C++, however, and I'm happy to recommend learning resources ranging across different levels of experience :)

For scenarios like this, where there's an unwritten rule about naming conventions, I like to create a test (or tests) to ensure any future changes follow the rule too.
How do others feel about this?
Such a test would iterate over all XAML files in the project and fail if any of the defined u:Id or x:Name values do not have the correct casing pattern.
Would such a test be appropriate in the 'CalculatorUnitTests' project or should it go somewhere else?

IMO, leveraging unit tests to enforce a style/naming convention feels a bit unorthodox. XAML Styler is used to enforce other rules in XAML files in the project; perhaps this is a useful feature for the that tool?

FYI - This cannot be solved with a simple regex and replace all. There are a ton of visual state triggers and style setters that will get erroneously overwritten.

It's a case of one at a time Find and Replace, taking care not to accidentally change AutomationIDs and Uids before those tasks are directly approached.

I'm about 1/2 way through the views now.

If you do know of a regex that works in VS2017 for matching: {x:Name"}{[A-Z\s+]}{[a-z0-9]*} to stop at camel-casing names, that would save me a little time, but not much. It's easy to click past x:Name= matches that are properly cased.

Hey Lance, this regex will find x:Names that are camel-cased: x:Name="[a-z] Or were you looking for something different?

That was my first try, but the quotes were messing with the regex-Find, when I removed it, I got no results. I figured it just me messing up the expression.

I'll try it again, but use Resharper for C++.

Works on my end:
image
image

It looks like when we went from private to public, something was broken with my private copy of the repo (_it was forked a while before this went public_). It removed linked commits, preventing me from opening a Pull Request and automatically unassigned me from this task.

To fix this, I had to re-fork the public repo and copy over my fixes to the latest master branch.

Unfortunately, this means all the work is going to be inside a single commit. However, this also allowed me to manually check for conflicts before pushing the commit.

Are we sure we want to change all x:Uid keys? How will the loc system handles this?

@jlaanstra has a good point. It's unfortunate that we wasted Lance's time making that change. @LanceMcCarthy, I apologize for that. We should revert the changes to the resw and focus on only updating the x:Names.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

grochocki picture grochocki  Â·  25Comments

MicrosoftIssueBot picture MicrosoftIssueBot  Â·  20Comments

imchau picture imchau  Â·  27Comments

ivadham picture ivadham  Â·  18Comments

MarcAnt01 picture MarcAnt01  Â·  14Comments