Generate INotifyPropertyChanged for Display property & Use global namespace for List type #38

Jack251970 · 2025-08-12T05:49:06Z

Generate INotifyPropertyChanged interface for Enum localize Display property

Since Display property will be changed with DemoLocalize.UpdateLabels method, we should implement INotifyPropertyChanged interface so that its change event will be notified.

Use global namespace for List type

Use global::System.Collections.Generic.List instead of using System.Collections.Generic; and List for code quality

…roperty

Copilot

Pull Request Overview

This PR enhances the EnumSourceGenerator to implement INotifyPropertyChanged for the Display property and uses global namespace references for List<T> types. The changes ensure proper change notification when Display property values are updated through the UpdateLabels method.

Implements INotifyPropertyChanged interface with proper event notification for the Display property
Replaces using directive with global namespace references for better code isolation
Updates the documentation reference from Flow.Launcher to Flow.Bar

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs

Co-authored-by: Copilot <[email protected]>

coderabbitai · 2025-08-12T05:54:33Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Generated enum data class now implements INotifyPropertyChanged, adds PropertyChanged and OnPropertyChanged, converts Display to a backing-field property that raises notifications, and fully-qualifies System types (including List) in signatures and usage.

Changes

Cohort / File(s)	Summary
Enum data class generation updates `Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs`	- Generated class now implements `global::System.ComponentModel.INotifyPropertyChanged` - Added `PropertyChanged` event and `OnPropertyChanged` method - `Display` converted to use private `_display` backing field and raises notifications in setter - Replaced `List<T>` with `global::System.Collections.Generic.List<T>` in signatures and construction (`GetValues`, `UpdateLabels`) - All System types in generated code are fully-qualified with `global::` (removes reliance on local `using` directives)

Sequence Diagram(s)

sequenceDiagram
  actor Consumer
  participant EnumData as GeneratedEnumData
  participant Subscribers as PropertyChangedSubscribers

  Consumer->>EnumData: set Display(newValue)
  EnumData->>EnumData: if value changed: _display = newValue
  EnumData->>Subscribers: OnPropertyChanged("Display")

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Improve Enum Localize Class Name & Add Useful Links and Documents #34 — touches the enum localization generator output and renames/generated-class-related changes; likely related to generated enum code modifications.

Suggested reviewers

Yusyuriv

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab674be and bacfa90.

📒 Files selected for processing (1)

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (5 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: gitStream.cm

🔇 Additional comments (4)

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (4)

201-201: INotifyPropertyChanged on the generated class: LGTM

The interface addition aligns with the PR intent and is correctly fully-qualified.

258-261: Fully-qualifying List in GetValues: LGTM

Return type and construction use global::System.Collections.Generic.List consistently.

274-275: No-op formatting change

Extra blank line; no functional impact.

341-341: Fully-qualifying List in UpdateLabels: LGTM

Signature aligns with the PR’s global namespace qualification goal.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch display_property_change

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (1)

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (1)
342-342: Consider widening UpdateLabels parameter to IEnumerable for broader applicability

You only enumerate items to update Display; accepting IEnumerable improves API flexibility without sacrificing behavior.

Apply this diff:
-            sb.AppendLine($"{tabString}public static void UpdateLabels(global::System.Collections.Generic.List<{enumDataClassName}> options)");
+            sb.AppendLine($"{tabString}public static void UpdateLabels(global::System.Collections.Generic.IEnumerable<{enumDataClassName}> options)");

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4edbef5 and ab674be.

📒 Files selected for processing (1)

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (5 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🔇 Additional comments (3)

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (3)

201-201: INotifyPropertyChanged on generated class — good upgrade

This aligns with the goal of notifying UI bindings when Display changes. Solid move.

258-261: Fully-qualifying List in generated API — consistent and clear

Using global::System.Collections.Generic.List in the generated code avoids relying on usings in the consumer and matches the PR objective.

274-275: No-op: whitespace

No functional change here.

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Yusyuriv · 2025-08-12T08:48:24Z

Does it make sense to remove using System.Collections.Generic now that you're specifying it on List directly now?

Jack251970 · 2025-08-12T08:54:08Z

Does it make sense to remove using System.Collections.Generic now that you're specifying it on List directly now?

Using global:: prefix can ensure the reference correction I think. If users introduce some other using like AAA.BBB by default with Using.cs file and there is a type called List, this will possibly cause conflict between System.Collections.Generic.List & AAA.BBB.List.

Microsoft.CommunityToolkit.MVVM also uses this to generate its IPropertyChanged interface. That is where I get this idea from. 🤔

Yusyuriv · 2025-08-12T09:34:10Z

I understand why. What I meant is now that you're explicitly specifying the namespace on each use, should we delete using System.Collection.Generic from the top of the generated file?

Jack251970 · 2025-08-12T09:37:14Z

I understand why. What I meant is now that you're explicitly specifying the namespace on each use, should we delete using System.Collection.Generic from the top of the generated file?

Hmm, I have deleted it already?

Jack251970 · 2025-08-12T09:37:33Z

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs

            GeneratedHeaderFromPath(sourceBuilder, enumFullName);
            sourceBuilder.AppendLine();

-            // Generate using directives


Using removed here

Yusyuriv · 2025-08-12T10:58:33Z

Sorry, I must have missed this. LGTM

Jack251970 added 2 commits August 12, 2025 13:40

Generate INotifyPropertyChanged interface for Enum localize Display p…

81cb149

…roperty

Use global namespace for List type

ab674be

github-actions bot assigned Jack251970 Aug 12, 2025

Jack251970 changed the title ~~Display property change~~ Generate INotifyPropertyChanged for Display property & Use global namespace for List type Aug 12, 2025

Jack251970 enabled auto-merge August 12, 2025 05:49

Jack251970 requested a review from jjw24 August 12, 2025 05:49

Jack251970 added the enhancement New feature or request label Aug 12, 2025

Jack251970 requested review from Yusyuriv, VictoriousRaptor, taooceros and Copilot August 12, 2025 05:50

Copilot AI reviewed Aug 12, 2025

View reviewed changes

Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs Outdated Show resolved Hide resolved

Fix typos

bacfa90

Co-authored-by: Copilot <[email protected]>

Jack251970 requested a review from Copilot August 12, 2025 05:53

coderabbitai bot reviewed Aug 12, 2025

View reviewed changes

Copilot AI reviewed Aug 12, 2025

View reviewed changes

Jack251970 commented Aug 12, 2025

View reviewed changes

Yusyuriv approved these changes Aug 12, 2025

View reviewed changes

Jack251970 merged commit f45b53f into main Aug 12, 2025
2 checks passed

Jack251970 deleted the display_property_change branch August 12, 2025 10:58

Jack251970 mentioned this pull request Aug 13, 2025

Enable nullable for enum localize classes & Bump version to 0.0.5 #39

Merged

Uh oh!

Generate INotifyPropertyChanged for Display property & Use global namespace for List type #38

Generate INotifyPropertyChanged for Display property & Use global namespace for List type #38

Uh oh!

Conversation

Jack251970 commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate INotifyPropertyChanged interface for Enum localize Display property

Use global namespace for List type

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

coderabbitai bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR/Issue comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Status, Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Yusyuriv commented Aug 12, 2025

Uh oh!

Jack251970 commented Aug 12, 2025

Uh oh!

Yusyuriv commented Aug 12, 2025

Uh oh!

Jack251970 commented Aug 12, 2025

Uh oh!

Jack251970 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Yusyuriv commented Aug 12, 2025

Uh oh!

Uh oh!

Uh oh!

Jack251970 commented Aug 12, 2025 •

edited

Loading

coderabbitai bot commented Aug 12, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)