More command line improvements (take 2) #2253
Merged
Conversation
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
Previously, two separate booleans (doUpload and doVerify) were used. However, since it always makes sense to specify only one of them, it makes more sense to keep a single action enum variable, which slightly simplifies the code (especially when more actions are added later). Additionally, an error is now shown when both --verify and --upload are specified on the commandline.
Previously, the code showed an error when the given action was not upload or verify. This is now reversed: the GUI is shown when the action is "GUI" (which is the default when no action specified). Since the action enum only contains these three values, there is no change in behaviour, but this makes it easier to add new actions later.
This uses a switch on the action value, which makes it more clear what code runs when. No actual behaviour is changed, most of the changes in this commit are indentation changes.
This method takes care of setting the serial.port preference to the given value, as well as deriving the serial.port.file preference. This should prevent duplicate code in the future. Note that a second copy of this code lives in SerialUploader, but that doesn't write to the global Preferences but a local prefs map. Since the global Preferences are currently static, there is no way to share code between these two copies.
This describes the versions where various options were introduced or changed.
Previously, the --board and --port arguments were stored in a variable first and only processed later. Now, the arguments are processed right away. This does mean that the arguments are processed when the GUI is not yet initialized, which caused problems with calling onBoardOrPortChange and friends from selectBoard. However, since the GUI is not initialized, there is no real reason to call them either - if we just set the preferences to the right values, the GUI will be initialized correctly later. For this reason, selectBoard no longer calls the GUI update methods. Instead, those are called from the GUI code when the board is changed through the menu instead (e.g., after calling selectBoard). This commit slightly changes behaviour. Previously, --board and --port only worked in combination with --verify and --upload, but were ignored when just starting the IDE. Now, these are processed regardless of the other options present. Additionally, this commit causes all changed preferences to be saved. Previously, only changes with --pref were saved, --board and --port options were only active for the current run. This was caused because the saving of the preferences happened as a side effect of loading the file in the Editor, but only the --pref option was processed at that time. Note that the --verbose options are still only active for the current run and are only valid combined with --verify or --upload (since they default to non-verbose instead of the current preference).
Since the handling of these options defaults to non-verbose (instead of the current preference), they make no sense when starting the IDE normally. Previously, these options would just be ignored in this case, now an error is shown.
Before, the preferences were saved as a side effect of loading files in the Editor, but it seems better to explicitely save them as well (this should prevent problems later on, if the Editor class is no longer used in --verify or --upload mode).
This allows setting preferences for the current run only, without remembering them for the next run. This is especially useful when combined with --verify or --upload.
Preferences.init would write out the default preferences when no preference file previously existed. This would cause a default preferences file to be written even when --no-save-prefs was passed, due to the ordering of things. However, since the Base constructor now already calls Preferences.save(), there is no need for Preferences.init to also do this. Since Base calls this after parsing the commandline, the --no-save-prefs option is now also properly respected.
Previously, --verbose would be processed after the preferences were saved, which should usually mean that it should never influence the saved preferences. However, if for whatever reason Preferences.save() would be called later, the verbosity preferences would still be messed up. Since we now have a Preferences.setDoSave() method, we can make sure that these verbosity preferences (and any other preferences that are changed after the build started) are never saved.
This option causes the IDE to process its commandline arguments and then quit. This allows setting preferences uses --pref, without having to also load the GUI or compile a sketch.
This allows reading specific preferences from the commandline.
Parsing commandline arguments inside Preferences isn't very elegant, this is better suited for the main function. Also, this change prepares for taking --curdir into account for --preferences-file as well.
Previously, the argument to --preferences-file would be interpreted as a filename, but then also checked as an option as well (in the next loop iteration). This didn't really matter in practice (unless you would be using a file called "--preferences-file"), but better skip the argument anyway.
This shouldn't change any behaviour, but prepares for upcoming changes.
This saves a few conversions from File object to String and is generally cleaner.
This method takes filenames as specified on the commandline and turns them into the right File object, taking into account the current directory passed through --curdir by the wrapper script.
On Windows, files are canonicalized to prevent issues with legacy 8.3 filenames. However, this canonicalization includes making the path absolute and this happened before applying --curdir to the path, making the latter a noop. By reversing the operations, this should allow both of them to do their work.
When a sketch looks like this:
Blink/
Blink.ino
Foo.ino
The idea is that opening Foo.ino should open up the sketch. However,
before this would show an error stating "The file Foo.ino needs to be
inside a sketch folder named Foo" instead.
This turned out to be due to a typo, which seems to have been present
for a long time. Note that when the main sketch file was a .pde file,
everything already worked as expected.
In a lot of places, (potentially) relative paths were passed to File without any processing, making them be resolved without taking into account --curdir. By passing them through Base.absoluteFile instead, these paths are resolved relative to the working directory before starting arduino (at least on Linux, which is currently the only platform supporting --curdir). This applies --curdir to the --preferences-file option and the build.path, settings.path, sketchbook.path preferences. For example, this now works as expected: arduino --pref build.path=build_dir --verify Blink.ino
Before, these were only shown in the GUI, which makes a failing commandline build a bit puzzling. As a side effect, the error is now shown in the log area in addition to the status line above the log area, but that should be ok.
When no build.path preference is present, a temporary directory is automatically created (and deleted). When a build.path was specified, but the directory does not exist, the IDE would show an error and fail to build, which is unexpected and not so friendly. This commit makes sure that the build directory is automatically created.
In the preferences dialog, the name of the preferences file is shown for advanced editing. If the filename is clicked, the folder containing the file is opened. However, this always used Base.getSettingsFolder, which is the folder where the settings file _normally_ resides. But when the --preferences-file option is used, the actual preferences file might be somewhere else. This commit makes sure to always open up the parent directory of the actual preferences file in use, instead of always the default one.
See arduino#2000 for details. Close arduino#2000
|
I had a look at the commit, it looks good. I haven't looked close enough to double-check it's correct, but it looks good at first glance. It seems a bit messy with all the flags, but I guess it's just tricky matter :-) One more thing: The manpage needs to be updated as well (in build/shared/arduino.adoc IIRC) |
|
You checked really quick: I've already updated the manpage as well :-) |
I don't think this is correct. IIRC Deleting the |
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.
See #2000 for details.