Support enum localization with EnumSourceGenerator & Support internal static PublicAPI instance with PublicApiSourceGenerator& Improve TabString performance #33
Conversation
Jack251970
changed the title
Jack251970
changed the title
Jack251970
changed the title
Jack251970
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for enum localization using a new EnumSourceGenerator and adds support for an internal static PublicAPI instance via PublicApiSourceGenerator. It includes new source generators that automatically generate code for enum data classes and plugin API access when dependency injection is enabled, along with corresponding diagnostics and helper modifications.
Reviewed Changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher.Localization.SourceGenerators/SourceGeneratorDiagnostics.cs | Added diagnostic FLSG0008 for invalid enum localization key/value. |
| Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs | Refactored plugin information handling and reporting diagnostics. |
| Flow.Launcher.Localization.SourceGenerators/Localize/PublicApiSourceGenerator.cs | Implements a source generator for creating an internal static PublicApi instance. |
| Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs | Updated to use dependency injection and renamed assembly variable for consistency. |
| Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs | Introduced enum localization code generation, including enum data classes and update methods. |
| Flow.Launcher.Localization.Shared/Helper.cs | Added a Spacing helper method for consistent code formatting. |
| Flow.Launcher.Localization.Shared/Constants.cs | Added new constant definitions for enum localization and PublicApi source generation. |
| Flow.Launcher.Localization.Attributes/* | Added attributes for marking enum localization keys/values and enums for localization. |
| Flow.Launcher.Localization.SourceGenerators/AnalyzerReleases.Unshipped.md | Documented new diagnostic FLSG0008. |
Files not reviewed (4)
- Flow.Launcher.Localization.Attributes/Flow.Launcher.Localization.Attributes.csproj: Language not supported
- Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj: Language not supported
- Flow.Launcher.Localization.slnx: Language not supported
- Flow.Launcher.Localization/Flow.Launcher.Localization.csproj: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs:337
- [nitpick] Consider updating the condition for UseLocalizationKey to use !string.IsNullOrWhiteSpace(LocalizationKey) instead of a null check, to ensure that an empty string does not mistakenly indicate a valid localization key.
public bool UseLocalizationKey => LocalizationKey != null;
📝 WalkthroughWalkthroughThe pull request introduces a suite of new localization components. Several attribute classes for enums and enum fields (for keys and values) have been added along with a dedicated project file for these attributes. Additionally, new constants and helper methods support localization functionality. Multiple source generators have been implemented or modified to generate enum data classes, validate plugin information, and produce a public API, with updates to diagnostic rules and project references included. Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler
participant ESG as EnumSourceGenerator
participant Attr as Attribute Checker
participant CodeGen as Source Generator
Compiler->>ESG: Provide enum declarations
ESG->>Attr: Filter enums with localization attributes
Attr-->>ESG: Return valid enum fields
ESG->>CodeGen: Generate enum data classes & methods
CodeGen-->>Compiler: Submit generated source code
sequenceDiagram
participant Compiler
participant PASG as PublicApiSourceGenerator
participant PluginHelper as PluginInfoHelper
Compiler->>PASG: Supply compilation context
alt Dependency Injection Enabled
PASG->>PluginHelper: Bypass plugin validation
else
PASG->>PluginHelper: Validate plugin info
PluginHelper-->>PASG: Return valid info or diagnostics
end
PASG->>Compiler: Generate public API source code
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj (1)
19-21:⚠️ Potential issueMissing reference to the new Attributes project.
According to the PR summary, this project should reference the newly created Flow.Launcher.Localization.Attributes project, but the reference is missing.
<ItemGroup> <ProjectReference Include="..\Flow.Launcher.Localization.Shared\Flow.Launcher.Localization.Shared.csproj" /> + <ProjectReference Include="..\Flow.Launcher.Localization.Attributes\Flow.Launcher.Localization.Attributes.csproj" /> </ItemGroup>The source generators need access to the attribute classes to properly identify and process decorated enums.
🧹 Nitpick comments (5)
Flow.Launcher.Localization.Shared/Helper.cs (1)
89-101: Consider simplifying the Spacing method implementation.While the current implementation works, it's unnecessarily complex. The method allocates a fixed-size character span, fills it with spaces, then copies the characters to a StringBuilder.
public static string Spacing(int n) { - Span<char> spaces = stackalloc char[n * 4]; - spaces.Fill(' '); - - var sb = new StringBuilder(n * 4); - foreach (var c in spaces) - { - _ = sb.Append(c); - } - - return sb.ToString(); + return new string(' ', n * 4); }The simplified version is more readable, performs better, and achieves the same result with less code.
Flow.Launcher.Localization.SourceGenerators/Localize/PublicApiSourceGenerator.cs (2)
10-17: Consider making the PackageVersion access more resilient.While the current implementation works, direct access to Assembly.GetName().Version can occasionally fail in certain build configurations.
-private static readonly Version PackageVersion = typeof(PublicApiSourceGenerator).Assembly.GetName().Version; +private static readonly Version PackageVersion = typeof(PublicApiSourceGenerator).Assembly.GetName().Version ?? new Version(1, 0, 0, 0);
99-102: Consider adding more comprehensive header information.The current header only includes the auto-generated marker. For consistency with other generators, consider adding more context similar to how GeneratedHeaderFromPath is implemented in LocalizeSourceGenerator.
-private static void GeneratedHeaderFromPath(StringBuilder sb) -{ - sb.AppendLine("/// <auto-generated/>"); -} +private static void GeneratedHeaderFromPath(StringBuilder sb) +{ + sb.AppendLine("/// <auto-generated>") + .AppendLine($"/// Generated by {nameof(PublicApiSourceGenerator)}") + .AppendLine("/// </auto-generated>"); +}Flow.Launcher.Localization.Shared/Constants.cs (2)
23-26: Introduce a small grouping or comment forEnumLocalize*constants.These newly added constants for enum localization (
EnumLocalizeClassSuffix,EnumLocalizeAttributeName,EnumLocalizeKeyAttributeName,EnumLocalizeValueAttributeName) are straightforward and aligned with the existing naming conventions. Consider grouping them under a short region or adding a summarizing comment to make their collective purpose clearer.
27-30: Fix the spelling in the inline comment for PublicApi constants.Line 27 contains the word "combiguity," which seems to be a misspelling of "ambiguity." A quick fix will improve readability.
Apply this diff to ensure clarity:
- // Use PublicApi instead of PublicAPI for possible combiguity with Flow.Launcher.Plugin.IPublicAPI + // Use PublicApi instead of PublicAPI to avoid ambiguity with Flow.Launcher.Plugin.IPublicAPI
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Flow.Launcher.Localization.Attributes/EnumLocalizeAttribute.cs(1 hunks)Flow.Launcher.Localization.Attributes/EnumLocalizeKeyAttribute.cs(1 hunks)Flow.Launcher.Localization.Attributes/EnumLocalizeValueAttribute.cs(1 hunks)Flow.Launcher.Localization.Attributes/Flow.Launcher.Localization.Attributes.csproj(1 hunks)Flow.Launcher.Localization.Shared/Constants.cs(1 hunks)Flow.Launcher.Localization.Shared/Helper.cs(2 hunks)Flow.Launcher.Localization.SourceGenerators/AnalyzerReleases.Unshipped.md(1 hunks)Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj(1 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs(1 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs(5 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/PublicApiSourceGenerator.cs(1 hunks)Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs(1 hunks)Flow.Launcher.Localization.SourceGenerators/SourceGeneratorDiagnostics.cs(1 hunks)Flow.Launcher.Localization.slnx(1 hunks)Flow.Launcher.Localization/Flow.Launcher.Localization.csproj(3 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs (2)
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (1)
ImmutableArray(133-204)Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (1)
ImmutableArray(109-161)
Flow.Launcher.Localization.Attributes/EnumLocalizeValueAttribute.cs (2)
Flow.Launcher.Localization.Attributes/EnumLocalizeAttribute.cs (1)
AttributeUsage(8-11)Flow.Launcher.Localization.Attributes/EnumLocalizeKeyAttribute.cs (4)
AttributeUsage(8-32)Equals(26-27)GetHashCode(29-29)IsDefaultAttribute(31-31)
Flow.Launcher.Localization.SourceGenerators/Localize/PublicApiSourceGenerator.cs (3)
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (3)
Generator(14-355)Initialize(31-54)Execute(61-103)Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (3)
Generator(19-641)Initialize(37-69)Execute(76-127)Flow.Launcher.Localization.Shared/Constants.cs (1)
Constants(5-34)
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (4)
Flow.Launcher.Localization.Shared/Constants.cs (1)
Constants(5-34)Flow.Launcher.Localization.Shared/Helper.cs (3)
GetFLLUseDependencyInjection(16-24)PluginClassInfo(34-61)Helper(12-104)Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs (2)
PluginClassInfo(10-78)PluginInfoHelper(8-79)Flow.Launcher.Localization.Shared/Classes.cs (2)
PluginClassInfo(5-28)PluginClassInfo(18-27)
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (4)
Flow.Launcher.Localization.Shared/Helper.cs (5)
Helper(12-104)PluginClassInfo(34-61)GetFLLUseDependencyInjection(16-24)Location(75-78)Location(80-83)Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs (2)
PluginClassInfo(10-78)PluginInfoHelper(8-79)Flow.Launcher.Localization.Shared/Constants.cs (1)
Constants(5-34)Flow.Launcher.Localization.SourceGenerators/SourceGeneratorDiagnostics.cs (1)
SourceGeneratorDiagnostics(6-79)
🔇 Additional comments (28)
Flow.Launcher.Localization.Attributes/Flow.Launcher.Localization.Attributes.csproj (1)
1-8: LGTM! The new project file is well-structured.The project file is correctly configured with appropriate target framework and namespace settings for attribute classes.
Flow.Launcher.Localization.slnx (1)
3-3: LGTM! Appropriate integration of the new project.The new Attributes project is correctly included in the solution file, maintaining consistency with the existing project structure.
Flow.Launcher.Localization.Shared/Helper.cs (1)
1-4: LGTM! Adding necessary namespace imports.The added namespace imports support the new functionality in the Helper class.
Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj (1)
7-8: LGTM! Proper version management.Adding the AssemblyVersion property is a good practice for package versioning.
Flow.Launcher.Localization.Attributes/EnumLocalizeAttribute.cs (1)
5-11: Well-designed attribute implementation.This attribute is correctly implemented as a marker attribute for enum localization. The
AttributeUsageconstraint properly restricts it to enum targets only, and the XML documentation clearly explains its purpose.Flow.Launcher.Localization.SourceGenerators/AnalyzerReleases.Unshipped.md (1)
14-14: Appropriate addition to unshipped analyzer rules.The new rule FLSG0008 follows the existing pattern and naming convention. It's properly categorized as a localization warning for invalid enum field localization key/value pairs.
Flow.Launcher.Localization.SourceGenerators/SourceGeneratorDiagnostics.cs (1)
71-78: Diagnostic descriptor correctly implemented.The new diagnostic descriptor follows the same pattern as existing ones. The message format, severity level, and category are appropriate for reporting issues with enum field localization.
Flow.Launcher.Localization/Flow.Launcher.Localization.csproj (3)
9-9: Verify the implications of changing DevelopmentDependency to false.Changing
DevelopmentDependencyfrom true to false means this package will be included as a regular dependency rather than a development-only dependency. Ensure this is intentional, as it affects how this package is treated by downstream projects.
36-38: Reference to new Attributes project correctly added.Adding a reference to the Attributes project with
IncludeAssetsset to "runtime" ensures that the attributes will be available at runtime, which is necessary for reflection-based operations on these attributes.
53-57: Appropriate packaging configuration for the Attributes library.Including the Attributes DLL in the lib folder is appropriate since these attributes need to be referenced by consuming projects at compile time and runtime. The
Visibleproperty is correctly set to true to make the assembly visible to projects that consume this package.Flow.Launcher.Localization.SourceGenerators/PluginInfoHelper.cs (1)
8-79: Well-structured validation logic with good separation of concerns.This helper class nicely encapsulates the plugin validation logic that was likely previously contained in the LocalizeSourceGenerator. The implementation follows a clear flow of validation steps with appropriate diagnostics reported at each stage.
Flow.Launcher.Localization.Attributes/EnumLocalizeKeyAttribute.cs (1)
5-32: Clean attribute implementation with proper equality and default handling.The attribute design follows best practices by:
- Specifying the correct AttributeUsage target
- Providing a default instance
- Implementing proper value-based equality
- Overriding GetHashCode consistently
- Implementing IsDefaultAttribute
The protected property pattern allows for potential extension while maintaining encapsulation.
Flow.Launcher.Localization.SourceGenerators/Localize/PublicApiSourceGenerator.cs (3)
25-34: LGTM! Clean initialization with proper incremental generator setup.The initialization method correctly sets up the dependencies required for the source generator.
41-54: Appropriate early exit pattern when dependency injection is disabled.The logic cleanly handles the dependency injection check and exits early when not needed, preventing unnecessary code generation.
60-97: Ensure null safety when retrieving the IPublicAPI service.While the source generator's produced code uses the null-coalescing assignment operator (
??=), it doesn't handle the case where GetRequiredService throws an exception if the service isn't registered.Consider verifying that your DI container documentation confirms that GetRequiredService throws an appropriate exception with a helpful message when the requested service isn't registered.
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (4)
100-100: Improved naming for better clarity.Renaming from
assemblyNametoassemblyNamespaceprovides better semantics, reflecting that the value represents a namespace rather than just an assembly name.
103-117: Clean conditional handling of plugin validation based on DI status.This change properly delegates plugin validation to the new PluginInfoHelper class and conditionally bypasses validation when using dependency injection, improving separation of concerns.
507-507: Good refactor to use the shared Helper.Spacing method.Moving the spacing functionality to a shared helper improves code reuse across the codebase.
513-514: Proper integration with the PublicApiSourceGenerator.This code correctly uses the PublicApi class generated by the PublicApiSourceGenerator when dependency injection is enabled, creating a well-designed coupling between the two generators.
Flow.Launcher.Localization.Attributes/EnumLocalizeValueAttribute.cs (1)
1-33: Attribute implementation looks solid.This new attribute class mirrors the structure and logic of
EnumLocalizeKeyAttributewith default instances, constructors, and equality overrides. It provides a clear and concise approach for marking localization values on enum fields. No correctness or security concerns stand out, and the usage ofIsDefaultAttribute()is consistent with best practices.Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs (8)
31-54: Efficient incremental generation approach inInitialize.The creation of syntax providers for enums and plugin classes is well-structured. The combination of these providers, along with config options and the compilation, is a clean approach to collecting relevant data for source generation. Good job leveraging incremental APIs here.
61-103: Cohesive logic inExecutemethod for orchestrating generation.You:
- Validate whether dependency injection is enabled.
- Retrieve valid plugin info if DI is disabled.
- Filter out any enums not tagged with
EnumLocalizeAttribute.- Tidy approach for conditionally generating sources.
Everything appears well-organized with proper diagnostic reporting for missing plugin info. No issues found.
109-161: Robust field extraction inGetEnumFields.The method cleanly identifies enum fields and gathers both key and value attributes. The fallback logic (reporting a diagnostic when neither key nor value is provided) is a nice fail-safe. Returning
_emptyEnumFieldson errors to prevent partial class generation is logical.
167-264: Comprehensive source generation inGenerateSource.The method:
- Builds a data class for each localizable enum using
StringBuilder.- Properly includes properties for the enum’s value, display text, localization key, and localization value.
- Generates a
GetValuesmethod reflecting all enum fields and callsGetEnumFieldsto ensure only valid fields are included.- Seamlessly injects references to either
PublicApi.Instance.GetTranslationorPluginInitContextbased on DI usage.The approach is well-encapsulated and maintains separation of concerns. Good use of the
StringBuilderto form the final output.
266-279: Header generation methodGeneratedHeaderFromPath.Produces a coherent autogeneration header, referencing the enum’s fully qualified name. Straightforward and helpful for debugging. Looks good as-is.
280-304: ConciseGenerateEnumFieldmethod with minimal duplication.The generation logic for each enum field is concise, properly distinguishing between key-based translations and direct localization values. Nicely handles the scenario when only a value is provided.
306-327: Effective label refresh logic inGenerateUpdateLabelsMethod.The loop checks for non-whitespace keys and reissues translations accordingly. This allows dynamic updates when locale or culture changes, which is crucial for a good user experience.
333-352:EnumFieldclass usage is well-structured.Holds essential metadata for an enum member, with clear property definitions. The
UseLocalizationKeylogic is intuitive, and the constructors handle both key-based and value-only fields neatly. No concerns here.
Jack251970
changed the title
There was a problem hiding this comment.
Copilot reviewed 11 out of 15 changed files in this pull request and generated no comments.
Files not reviewed (4)
- Flow.Launcher.Localization.Attributes/Flow.Launcher.Localization.Attributes.csproj: Language not supported
- Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj: Language not supported
- Flow.Launcher.Localization.slnx: Language not supported
- Flow.Launcher.Localization/Flow.Launcher.Localization.csproj: Language not supported
There was a problem hiding this comment.
Copilot reviewed 11 out of 15 changed files in this pull request and generated 2 comments.
Files not reviewed (4)
- Flow.Launcher.Localization.Attributes/Flow.Launcher.Localization.Attributes.csproj: Language not supported
- Flow.Launcher.Localization.SourceGenerators/Flow.Launcher.Localization.SourceGenerators.csproj: Language not supported
- Flow.Launcher.Localization.slnx: Language not supported
- Flow.Launcher.Localization/Flow.Launcher.Localization.csproj: Language not supported
Flow.Launcher.Localization.SourceGenerators/Localize/EnumSourceGenerator.cs
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
Can it be used with |
Yes, we can do that. Tested as follows: 
|
Important
I have thoroughly tested the
EnumSourceGeneratorandPublicApiSourceGeneratorand I believe them are ready to move to production.Support
enumlocalization withEnumSourceGeneratorEnum localization solution is from https://github.com/Flow-Launcher/Flow.Launcher/blob/dev/Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs.
If we have files with this enum declarition:
Generator will generate outputs as follows.
If we set
FLLUseDependencyInjectionto false,Else If we set
FLLUseDependencyInjectionto true,Then, in view model, you can get
ItemSourceandSelectedValuelike this:In page, you can bind them like this:
Test
Support Internal static PublicAPI instance with PublicApiSourceGenerator
When enabling
FLLUseDependencyInjection, we will getIPublicAPIinstance fromIoc.Default. We can put it in internal field forLocalizeclass and enum data classes to use.Test
Will generate file like this:
And
Localizeclass is like this:And enum data classes are shown as above.