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

Refactor Compile.ps1 Script for Modularity and Enhanced Functionality #2587

Closed
wants to merge 4 commits into from

Conversation

fam007e
Copy link
Contributor

@fam007e fam007e commented Aug 19, 2024

Type of Change

  • Refactoring
  • New feature
  • Documentation update

Description

This pull request refactors the Compile.ps1 script with the following key improvements:

  • Moved Update-Progress Function:

    • The Update-Progress function has been moved to an external script for better modularity and reusability.
  • Added Encode-JsonSpecialChars Function:

    • A new function, Encode-JsonSpecialChars, has been introduced to handle JSON special character encoding more effectively. This improves the robustness of JSON handling and prevents potential formatting issues.
  • Implemented Error Handling:

    • Error handling has been enhanced with try/catch blocks during preprocessing and compilation steps, ensuring that issues are caught and handled gracefully.
  • Introduced Logging Functionality:

    • Logging has been added to track progress and capture errors during the script execution. This provides better traceability and helps in debugging.
  • Improved Path Handling:

    • Paths are now handled using $PSScriptRoot and Join-Path for better portability across different environments. This change improves the script's reliability when run in various directory structures.

Testing

  • The script was tested in both Windows PowerShell and PowerShell Core environments.
  • The -Debug, -Run, and -SkipPreprocessing switches were tested to confirm that they function as expected.
  • The syntax validation feature was tested to ensure that it catches common errors in the generated script.
  • Logging was verified to ensure that all steps and errors are properly captured.

Impact

  • The refactoring enhances the maintainability and readability of the Compile.ps1 script.
  • The improved modularity allows for easier updates and potential reusability of components in other scripts.
  • The introduction of error handling and logging makes the script more robust and easier to debug.

Issue related to PR

  • Resolves #

Additional Information

  • This refactoring is part of a larger effort to improve the overall code quality of the project. Future updates may include further modularization or additional features to enhance functionality.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no errors/warnings/merge conflicts.

…ments

- Moved Update-Progress function to an external script for better modularity.
- Added Encode-JsonSpecialChars function to handle JSON special character encoding.
- Implemented error handling with try/catch blocks during preprocessing and compilation.
- Introduced logging functionality to track progress and errors.
- Improved path handling using $PSScriptRoot and Join-Path for better portability.
@Real-MullaC
Copy link
Contributor

This only changes 1 file and does not add any files.

Compile.ps1 Show resolved Hide resolved
tools/Update-Progress.ps1 Outdated Show resolved Hide resolved
tools/Update-Progress.ps1 Outdated Show resolved Hide resolved
Copy link
Contributor

@og-mrk og-mrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fam007e Because of the new changes introduced in this PR, new .log files will be created when running Compile.ps1 script.. which might accidently be added in the future, and because these logs are for error checking/diagnosis.. It's a good to ignore (not track) these files by git (the version management system used by this project, and every project on GitHub).

To do so (ignore .log files) you can add the following line in .gitignore file:

*.log

Copy link
Contributor

@og-mrk og-mrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After further testing, turns out that these changes have several problems, major one is the json parsing part (it appears as if doesn't do its job correctly), resulting in Syntax Errors.

Plus there's currently a bug in Invoke-Preprocessing script, where it doesn't test the possibility of an absolute file paths in Exclude Files List.. resulting in false positives in the validation step, I'll resolve this bug in PR #2579.

@og-mrk
Copy link
Contributor

og-mrk commented Aug 23, 2024

@fam007e I've fixed in PR 1 (on your fork) the issues described in this review comment. Please consider reviewing my PR over on your fork, and test my changes after merging it. Thanks in advance 😄

og-mrk added a commit to og-mrk/winutil that referenced this pull request Aug 24, 2024
Thanks to  @fam007e for improving this section in his PR ChrisTitusTech#2587 changes

Co-authored-by: fam007e <[email protected]>
@og-mrk
Copy link
Contributor

og-mrk commented Aug 24, 2024

@fam007e I would like to say, first and foremost.. thanks for your contribution, it's awesome to see others contribute to WinUtil, no matter how simple/complex their changes might be...

With that being said, and after some changes done to PR #2465 by @MyDrift-user (a somewhat medium sized Refactoring PR).. so the changes in this PR are not needed. I hope this doesn't discourages you from contributing to this project or other open source projects.. as WinUtil recently is in active development from several contributors, so conflict in interests may arise.

I recommend waiting to see @ChrisTitusTech opinion on this, but from a technical stand point these changes of yours will cause some problems, not to mention you'll need to update Compile.ps1 script, that's.. if PR #2465 is merged in the first place.

EDIT: Please note changes related to separation of Update-Progress won't be affected, and I do think it's a good idea to re-use code, as Update-Progress is Copy-Pasted in tools/devdocs-generator.ps1 script as well 😅

MyDrift-user pushed a commit to MyDrift-user/winutil that referenced this pull request Aug 24, 2024
* Remove the Special Character Escaping for Json Files as there's no need for it anymore

* Simplify 'application.json' Json Prefix Addition in 'Compile.ps1' Script

Thanks to  @fam007e for improving this section in his PR ChrisTitusTech#2587 changes

Co-authored-by: fam007e <[email protected]>

---------

Co-authored-by: fam007e <[email protected]>
@fam007e fam007e closed this Aug 26, 2024
ChrisTitusTech added a commit that referenced this pull request Aug 28, 2024
…lation (#2629)

* try1: apps panel

* refractor

- removed Get-TabXaml.ps1
- removed compilation part from compile.ps1
- removed existing changes from main.ps1
- added Invoke-WinUtilUIElements.ps1
- refractor existing changes into new function
- modified inputXML to call function

* removed unused replacementlogics

* fix toggle style

* move naming from winutil to wpf

* hotfixes

- reworked function calling
- working on correct theming support

* hotfix

* hotfix

fix missing elements
fix var naming

* some fixes

* hotfixes

* add image logic

* fix ? issue in search

* filter out unneeded categories

* cleanup

* fix border styling

* fixes

* preprocessing

* fix typo

* last fixes & add notes

* Update functions/public/Invoke-WPFUIElements.ps1

Co-authored-by: Mr.k <[email protected]>

* fix margin

* fix tabcontent margin

* fix some other margin issues

* move fixed values outside of iterations

* little fix

* add Dispatcher.Invoke

* add error handling for styles

* Update functions/public/Invoke-WPFUIElements.ps1

Co-authored-by: Mr.k <[email protected]>

* remove dispatcher

* fix search

* move run & undo tweaks to be fixed

* add error handling

* fix throw exception

* fixed accidental removal of findname grid call

* add padding & margin to make search look good

* remove grid to make it look correctly on small window

* fix rectangle

* Compiler Improvements for PR #2465 (#7)

* Remove the Special Character Escaping for Json Files as there's no need for it anymore

* Simplify 'application.json' Json Prefix Addition in 'Compile.ps1' Script

Thanks to  @fam007e for improving this section in his PR #2587 changes

Co-authored-by: fam007e <[email protected]>

---------

Co-authored-by: fam007e <[email protected]>

* fix margin of search clear

* fix cursor on clear search button

* undo fixed run & undo

* refractor themes.json

* undo themes.json

---------

Co-authored-by: Mr.k <[email protected]>
Co-authored-by: fam007e <[email protected]>
Co-authored-by: Chris Titus <[email protected]>
@ChrisTitusTech ChrisTitusTech added the skip-changelog Skip Change Logs label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skip Change Logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants