Fix ElectronSingleInstance handling #996
Conversation
softworkz
changed the title
softworkz
force-pushed
the
submit_singleinstance
branch
from
c1fa662 to
30037fc
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the ElectronSingleInstance configuration from a string-based approach (using "yes"/"no" values) to a proper boolean type throughout the codebase. The change simplifies the logic and makes the configuration more type-safe.
Key changes:
- Changed
ElectronSingleInstancefrom string to bool type in the data model - Updated the JSON template to use boolean values instead of quoted strings
- Simplified JavaScript condition checking from string comparison to boolean evaluation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ElectronNET.API/Runtime/Data/BuildInfo.cs | Changed ElectronSingleInstance property type from string to bool |
| src/ElectronNET.API/Runtime/StartupManager.cs | Simplified parsing logic to directly assign boolean value instead of mapping to "yes"/"no" strings |
| src/ElectronNET/build/ElectronNET.LateImport.targets | Added .ToLower() to convert boolean value to lowercase string representation for JSON template |
| src/ElectronNET/build/package.template.json | Removed quotes around $(ElectronSingleInstance) to make it a JSON boolean instead of string |
| src/ElectronNET.Host/main.js | Changed condition from string comparison (=== "yes") to direct boolean evaluation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| buildInfo.ElectronSingleInstance = "no"; | ||
| buildInfo.ElectronSingleInstance = parsedBool; | ||
| } |
There was a problem hiding this comment.
When ElectronSingleInstance is not set or is an empty string, the bool.TryParse will fail and ElectronSingleInstance will remain as its default value (false). However, the default in ElectronNET.LateImport.targets is "true" (line 110). This creates an inconsistency where an unset property would result in false in C# but true in the package.json. Consider adding an else clause to set a default value of true to match the MSBuild default, or ensure the property is always explicitly set.
| } | |
| } | |
| else if (string.IsNullOrEmpty(isSingleInstance)) | |
| { | |
| // Match MSBuild default in ElectronNET.LateImport.targets when the property is unset | |
| buildInfo.ElectronSingleInstance = true; | |
| } |
There was a problem hiding this comment.
No, this is not correct. The default is only in the props file, but what matters is the value at the targets level (after everything that is defined in the .csproj file has been applied).
The value at the targets level is what goes into the metadata attribute and that's the same value that is used during compilation.
So this suggestion is totally wrong.
pr-comment: Run #16
🎉 All tests passed!Github Test Reporter by CTRF 💚 |
2 participants
Supersedes #994