Update System.CommandLine dependency #776
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| command.AddOption(oInput); | ||
| command.AddOption(oChannel); | ||
| command.Handler = CommandHandler.Create((string input, AppReleaseChannel channel) => | ||
| var oChannel = new Option<AppReleaseChannel>("--channel", "-c") |
There was a problem hiding this comment.
Make this in Aliases style?
Co-authored-by: Shatyuka <shatyuka@qq.com>
|
|
||
| var e = new ArgumentException("Argument to run this command is invalid! See the information above for more detail."); | ||
| Console.WriteLine(e); | ||
| m_appMode = AppMode.Launcher; | ||
| } | ||
|
|
||
| public static void ParseHi3CacheUpdaterArguments(params string[] args) | ||
| private static void ParseHi3CacheUpdaterArguments(params string[] _) |
There was a problem hiding this comment.
Can't the args parameter be removed in all of the Parse* functions as it's never used?
1. Sentry 2. IconExtract 3. AppPreset 4. Velopack 5. Set CD 6. AppSettings
Co-authored-by: Gabriel Lima <44784408+gablm@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Updates the System.CommandLine packages to beta 5 and modernizes several components of the launcher.
- Bumps System.CommandLine and NamingConventionBinder to 2.0.0-beta5
- Refactors argument parsing to the new Option/CommandHandler API
- Improves console allocation logic and skips redundant metadata writes
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Hi3Helper.Win32 | Advanced submodule commit pointer |
| LoggerConsole.cs | Added AttachConsole fallback, unified allocation and error handling |
| CollapseLauncher/packages.lock.json | Updated System.CommandLine versions to beta 5 |
| CollapseLauncher/Program.cs | Relocated Sentry SDK initialization and updater hook |
| CollapseLauncher/CollapseLauncher.csproj | Switched package versions and added Debug OutputType override |
| CollapseLauncher/Classes/Properties/ArgumentParser.cs | Migrated to new System.CommandLine API and cleaned up parsing |
| CollapseLauncher/Classes/Extension/VelopackLocatorExtension.cs | Skip writing metadata if unchanged |
Comments suppressed due to low confidence (1)
CollapseLauncher/CollapseLauncher.csproj:95
- Overriding OutputType to Exe in the Debug configuration will conflict with the main WinExe setting; consider removing this or aligning it with the intended output type.
<OutputType>Exe</OutputType>
|
|
||
| if (!isConsoleApp && !PInvoke.AllocConsole()) | ||
| if (!isConsoleApp && !PInvoke.AttachConsole(0xFFFFFFFF)) |
There was a problem hiding this comment.
Replace the literal 0xFFFFFFFF with a named constant (e.g. ATTACH_PARENT_PROCESS) to clarify its meaning.
| if (!isConsoleApp && !PInvoke.AttachConsole(0xFFFFFFFF)) | |
| const uint ATTACH_PARENT_PROCESS = 0xFFFFFFFF; | |
| if (!isConsoleApp && !PInvoke.AttachConsole(ATTACH_PARENT_PROCESS)) |
Copilot uses AI. Check for mistakes.
| private static void AddHi3CacheUpdaterOptions() { } | ||
|
|
There was a problem hiding this comment.
The AddHi3CacheUpdaterOptions method is empty, so no options are added to the hi3cacheupdate command; it should register the required options and handler.
| private static void AddHi3CacheUpdaterOptions() { } | |
| private static void AddHi3CacheUpdaterOptions() | |
| { | |
| var oCachePath = new Option<string>("--cache-path") | |
| { | |
| Description = "Path to the cache directory", | |
| Required = true | |
| }; | |
| var oUpdateMode = new Option<string>("--update-mode") | |
| { | |
| Description = "Mode of cache update (e.g., 'full', 'incremental')", | |
| Required = true | |
| }; | |
| var hi3CacheUpdateCommand = _rootCommand.Children.OfType<Command>().FirstOrDefault(c => c.Name == "hi3cacheupdate"); | |
| if (hi3CacheUpdateCommand != null) | |
| { | |
| hi3CacheUpdateCommand.Options.Add(oCachePath); | |
| hi3CacheUpdateCommand.Options.Add(oUpdateMode); | |
| hi3CacheUpdateCommand.Action = CommandHandler.Create((string cachePath, string updateMode) => | |
| { | |
| m_arguments.Hi3CacheUpdater = new ArgumentHi3CacheUpdater | |
| { | |
| CachePath = cachePath, | |
| UpdateMode = updateMode | |
| }; | |
| }); | |
| } | |
| } |
Copilot uses AI. Check for mistakes.
| { | ||
| command.SetHandler(() => | ||
| var oInput = new Option<string>("--output-path") |
There was a problem hiding this comment.
The updater option is declared as --output-path but the handler signature expects a parameter named input, causing a binding mismatch; rename the option or the handler parameter to match.
Copilot uses AI. Check for mistakes.
| var command = new Command("takeownership", "Take ownership of the folder"); | ||
| command.AddOption(inputOption); | ||
| command.Handler = CommandHandler.Create((string input) => | ||
| var inputOption = new Option<string>("--input-path") |
There was a problem hiding this comment.
The takeownership command defines --input-path but the handler expects input, leading to a name mismatch; align the option name with the handler parameter.
| var inputOption = new Option<string>("--input-path") | |
| var inputOption = new Option<string>("--input") |
Copilot uses AI. Check for mistakes.
|
Main Goal
Update System.CommandLine to beta 5
What works:
open, though it doesn't state the error or notice when the argument input is invalid.traygeneratevelopackmetadataPR Status :
Templates
Changelog Prefixes