-
-
Notifications
You must be signed in to change notification settings - Fork 108
fix(cli): support sketches with custom main file names #1329
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
base: main
Are you sure you want to change the base?
Conversation
Previously, the CLI only accepted sketches where the main .pde file matched the sketch folder name (e.g., sketch/sketch.pde). This caused issues when users renamed their main file in the IDE, which stores the custom filename in sketch.properties. Now the CLI checks sketch.properties for a 'main' property before falling back to the default naming convention, matching the IDE's behavior implemented in Sketch.findMain(). Fixes processing#1219
|
@Stefterv, kindly review this too when you get a chance |
|
Hi @avinxshKD the code looks great to me, the tests however could use some work. Could you please create a test that tests the actual issue? See https://github.com/processing/processing4/blob/main/app/test/processing/app/CLITest.kt on how to write tests for the Command Line Interface! |
Added testSketchWithCustomMainFile() to CLITest.kt as requested by maintainer. This test provides a placeholder for manual testing of sketches with custom main files specified in sketch.properties. Follows the same pattern as existing CLI tests (testLSP, testLegacyCLI) and is intended to be run manually in IntelliJ IDEA.
Hi @Stefterv, thank you for the feedback! I've made the following updates:
The CLI test is ready for manual verification once you have a chance to test with a sketch that has a custom main file in |
|
Hi @avinxshKD please see here on how to create a temporary directory and temporary sketch so this does not need to be a manual test. We do not do any manual testing unfortunately processing4/app/test/processing/app/SketchbookTest.kt Lines 16 to 23 in ef226c9
|
Converted testSketchWithCustomMainFile() from manual to automated test. Now creates a temporary sketch folder with custom main file and sketch.properties, then tests the CLI build command. Follows the pattern from SchemaTest.kt using Files.createTempDirectory() and automatic cleanup.
|
Hi @Stefterv, thank you for the guidance I've updated the test to be fully automated as requested |
Problem
CLI rejected sketches with custom main file names (e.g.,
sketch/main.pde) even though the IDE supports them viasketch.properties.Solution
Updated CLI to check
sketch.propertiesfor amainproperty before falling back to default naming, matching IDE behavior fromSketch.findMain().Changes
Commander.java: Added Settings import and custom main file detection logicCommanderTest.java: Added unit tests for default and custom main file handlingFixes #1219