Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSCode clean-up and other minor fixes #3920

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

HebaruSan
Copy link
Member

Motivation

I had been using VSCodium for a while and recently decided to try the full VSCode. It suggested installing a C# plugin (which I eventually swapped out for OmniSharp), which in turn pointed out a bunch of helpful coding style changes. Addressing these will make it easier to spot new suggestions when working on new code.

For now I have opted not to address the alerts related to property and method names starting with lowercase letters or underscores (with a few easy exceptions).

Changes

  • To allow VSCode to give advice on GUI and Netkan, their csproj files are updated to Sdk format (in which files are added implicitly by MSBuild instead of being listed explicitly). GUI needed a complicated Update= rule to set the DependentUpon property for its .resx files, but this has made the GUI csproj much shorter and will drastically simplify future translation additions.
  • Unused usings are removed
  • The readonly keyword is added to many fields that are only set in the constructor
  • Properties set after new are moved into { Property = Value} blocks
  • Variables set by out params are now declared inline, and unused ones now use out _ to discard the value
  • Some unused fields and parameters are removed
  • Variables that previously were set to an as expression and checked for null are now handled with is-based pattern matching
  • Some conditional expressions are simplified (e.g., a ternary using a true or false constant replaced with an equivalent expression using || or &&)
  • Some expressions dealing with nullable values are simplified (e.g., replacing a ternary expression with a ?? expression)
  • Event invocations that check null first have been replaced by ?.Invoke() calls

Along the way a few more opportunities for improvement were indulged:

  • /.vscode/ is added to .gitignore so I can save a settings file there indicating that the default solution is CKAN.sln in the root rather than Tests/Data/CKAN.sln
  • Some references are updated a few minor or patch versions
  • In ConsoleUI's mod info screen, the dependency list will no longer have an extra blank line between every mod when a scrollbar is present
  • The repeated parts of ManageMods.CompareColumn are refactored into a generic CompareToNullable function
  • ThemedTabControl's brush is now disposed, as is necessary according to a comment in FlatToolStripRenderer
  • GUIConfiguration.FixColumnName still had hard coded names where its parameters were supposed to be used, so some of our column renaming logic may not have been working. This is now fixed.
  • A stray mid-file BOM is removed
  • The long-dead XBuild is no longer mentioned in the description of the Build-DotNet task

I'm going to self-review this because the changes, though numerous, are trivial and should not change any functionality.

@HebaruSan HebaruSan added Easy This is easy to fix GUI Issues affecting the interactive GUI Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Netkan Issues affecting the netkan data Tests Issues affecting the internal tests ConsoleUI Issues affecting the interactive console UI labels Oct 18, 2023
@HebaruSan HebaruSan merged commit b50dda0 into KSP-CKAN:master Oct 18, 2023
10 checks passed
@HebaruSan HebaruSan deleted the fix/vscode-cleanup branch October 18, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line ConsoleUI Issues affecting the interactive console UI Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix GUI Issues affecting the interactive GUI Netkan Issues affecting the netkan data Tests Issues affecting the internal tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant