-
-
Notifications
You must be signed in to change notification settings - Fork 41
Fix LT-22265: If there are new 'common' columns show them by default #486
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes a user interface issue where new 'common' columns in XML browse views were not being displayed by default. The changes ensure that when loading column configurations, any new common columns that weren't previously saved are automatically added to the visible columns list.
- Tracks which possible columns are new (not in saved configuration)
- Automatically shows new columns marked as 'common' by default
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| foreach (XmlNode node in doc.DocumentElement.SelectNodes("//column")) | ||
| { | ||
| // if there is a corresponding possible column remove it from the newPossibleColumns list. | ||
| var possible = newPossibleColumns.Find(n => n.Attributes?["label"].Value == node.Attributes?["label"].Value); |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NullReferenceException if node.Attributes is null. The null-conditional operator protects against node.Attributes being null, but not against node.Attributes["label"] returning null when the "label" attribute doesn't exist.
| var possible = newPossibleColumns.Find(n => n.Attributes?["label"].Value == node.Attributes?["label"].Value); | |
| var possible = newPossibleColumns.Find(n => | |
| n.Attributes != null && | |
| n.Attributes["label"] != null && | |
| node.Attributes != null && | |
| node.Attributes["label"] != null && | |
| n.Attributes["label"].Value == node.Attributes["label"].Value); |
| var newPossibleColumns = new List<XmlNode>(m_possibleColumns); | ||
| foreach (XmlNode node in doc.DocumentElement.SelectNodes("//column")) | ||
| { | ||
| // if there is a corresponding possible column remove it from the newPossibleColumns list. |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comma in comment. Should be 'If there is a corresponding possible column, remove it from the newPossibleColumns list.'
| // if there is a corresponding possible column remove it from the newPossibleColumns list. | |
| // if there is a corresponding possible column, remove it from the newPossibleColumns list. |
253ee7a to
4aa79c8
Compare
aror92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aror92 reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @jasonleenaylor)
This change is