Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new hereby diff task to streamline the workflow for comparing baseline test results. The task allows developers to use their preferred external diff tool (specified via the DIFF environment variable) to visually compare reference baselines against locally generated baselines.
Changes:
- Added a
getDiffTool()helper function to retrieve and validate theDIFFenvironment variable - Added a
difftask that invokes the external diff tool with reference and local baseline directories
| function getDiffTool() { | ||
| const program = process.env.DIFF; | ||
| if (!program) { | ||
| console.warn("Add the 'DIFF' environment variable to the path of the program you want to use."); |
There was a problem hiding this comment.
The error message is unclear and grammatically awkward. It should directly state what the user needs to do. Consider rephrasing to: "DIFF environment variable is not set. Set it to the path of your preferred diff tool (e.g., 'export DIFF=meld')."
| console.warn("Add the 'DIFF' environment variable to the path of the program you want to use."); | |
| console.warn("DIFF environment variable is not set. Set it to the path of your preferred diff tool (e.g., 'export DIFF=meld')."); |
| function getDiffTool() { | ||
| const program = process.env.DIFF; | ||
| if (!program) { | ||
| console.warn("Add the 'DIFF' environment variable to the path of the program you want to use."); |
There was a problem hiding this comment.
Using console.warn followed by process.exit(1) is inconsistent with error handling patterns in the codebase. For fatal errors, either throw an Error (like assertTypeScriptCloned at line 148) or use console.error. Since this is a fatal error that exits the process, it should use console.error instead of console.warn.
| console.warn("Add the 'DIFF' environment variable to the path of the program you want to use."); | |
| console.error("Add the 'DIFF' environment variable to the path of the program you want to use."); |
I was setting up a new environment and realized this was missing, and I'd been relying on saved paths in my difftool to do diffs.