Refactoring/cleanup for IlSpyCmd to Prepare for Multithreading (Stacked PR #4)#5
Open
petercrabtree wants to merge 5 commits into
Open
Refactoring/cleanup for IlSpyCmd to Prepare for Multithreading (Stacked PR #4)#5petercrabtree wants to merge 5 commits into
petercrabtree wants to merge 5 commits into
Conversation
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
from
c44f4d6 to
d0725ab
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
d0d159e to
610123b
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
from
d0725ab to
a234f45
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
610123b to
6d984e9
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
2 times, most recently
from
a68086b to
b091a3f
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
6d984e9 to
aebe02a
Compare
petercrabtree
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and cleans up the ILSpyCmd codebase to prepare for multithreading support. The changes focus on code organization, parameter handling, and method extraction to improve maintainability.
- Refactors the main execution flow by extracting methods and improving parameter passing
- Updates code style and formatting for better consistency
- Adds ReSharper settings and improves error handling patterns
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ILSpy.sln.DotSettings | Adds ReSharper configuration for code formatting and custom dictionary entries |
| ValidationAttributes.cs | Updates validation error message for better clarity |
| IlspyCmdProgram.cs | Major refactoring of the main program class with method extraction and improved parameter handling |
| DotNetToolUpdateChecker.cs | Adds explanatory comment for exception handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (obj.CreateCompilableProjectFlag && string.IsNullOrEmpty(obj.OutputDirectory)) | ||
| { | ||
| return new ValidationResult("--project cannot be used unless --outputdir is also specified"); | ||
| return new ValidationResult("you must specify --outputdir (-o) when using --project (-p)"); |
There was a problem hiding this comment.
The error message uses lowercase 'you' which is inconsistent with typical error message conventions. Consider using 'You must specify...' or a more formal structure like 'Output directory (-o) must be specified when using --project (-p)'.
Suggested change
| return new ValidationResult("you must specify --outputdir (-o) when using --project (-p)"); | |
| return new ValidationResult("Output directory (--outputdir, -o) must be specified when using --project (-p)."); |
Comment on lines
+276
to
+281
| if (outputDirectory == null) | ||
| { | ||
| var values = EntityTypes.SelectMany(v => v.Split(',', ';')).ToArray(); | ||
| HashSet<TypeKind> kinds = TypesParser.ParseSelection(values); | ||
| if (outputDirectory != null) | ||
| { | ||
| string outputName = Path.GetFileNameWithoutExtension(fileName); | ||
| output = File.CreateText(Path.Combine(outputDirectory, outputName) + ".list.txt"); | ||
| } | ||
|
|
||
| return ListContent(fileName, output, kinds); | ||
| await app.Error.WriteLineAsync("Output directory (-o) must be specified when creating a project (-p)."); | ||
| return ProgramExitCodes.EX_USAGE; | ||
| } | ||
| else if (ShowILCodeFlag || ShowILSequencePointsFlag) | ||
|
|
There was a problem hiding this comment.
This validation check is redundant because the ProjectOptionRequiresOutputDirectoryValidation attribute already validates this condition at the command-line parsing level. The validation will prevent execution from reaching this point when the condition fails.
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
aebe02a to
7b4461b
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
2 times, most recently
from
7f9cc0b to
dca1a8b
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
7b4461b to
bcc21db
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
bcc21db to
d396a02
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
from
dca1a8b to
07922a9
Compare
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
d396a02 to
d1f76ae
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
3 times, most recently
from
cf305a6 to
137f93e
Compare
By passing around instances explicitly in methods, it makes safely refactoring into parallel decompilation easier, because it's easy to find every instance of anything that needs to be duplicated for each thread. refactor: Extract RunAsync method from OnExecuteAsync Making dependencies more explicit by passing parameters instead of using instance fields. Inlines ResolveOutputDirectory and UpdateSettings helper methods for simplicity.
petercrabtree
force-pushed
the
dev/dev-env-clean-no-bom
branch
from
b3b3fa1 to
b4fb59a
Compare
petercrabtree
force-pushed
the
feature/multithread-ilspycmd-prep
branch
from
137f93e to
8103b3e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I decided to split this out to make reviewing easier.