Skip to content

Fix unneeded "Select port on upload" message #8194

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Nov 14, 2018
Merged

Fix unneeded "Select port on upload" message #8194

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
205ce59
Removed dead-code in comments
cmaglie Nov 9, 2018
eefe8dd
Refactor: simplified TargetBoard retrieve
cmaglie Nov 9, 2018
0310ec1
Refactor: simplified bool prefs retrieval
cmaglie Nov 9, 2018
aebdba2
Refactor: removed useless parameter from StringReplacer.formatAndSplit
cmaglie Nov 9, 2018
6ea66ea
Refactor: Removed code duplication in Export(App)Handlers
cmaglie Nov 9, 2018
2cf63e7
Added StringReplacer.checkIfRequiredKeyIsMissingOrExcept helper method
cmaglie Nov 9, 2018
459c173
Fix 'Select port on upload' error message
cmaglie Nov 9, 2018
89ff7b2
Show port on status bar only when selected
cmaglie Nov 9, 2018
d903d6b
Nicer serial port selection dialog box
cmaglie Nov 9, 2018
c1c8d5f
Editor: the serial port selection dialog now works
cmaglie Nov 12, 2018
13e72be
Editor: fixed port ordering in tools menu
cmaglie Nov 12, 2018
d99a31c
Better error for 'no serial ports available'
cmaglie Nov 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions arduino-core/src/processing/app/helpers/StringReplacer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,50 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;

public class StringReplacer {

public static void checkIfRequiredKeyIsMissingOrExcept(String key, String src, PreferencesMap inDict) throws PreferencesMapException {
// If the key is not missing -> everything is OK
String checkedValue = inDict.get(key);
if (checkedValue != null && !checkedValue.isEmpty())
return;

PreferencesMap dict = new PreferencesMap(inDict);

// Find a random tag that is not contained in the dictionary and the src pattern
String tag;
while (true) {
tag = UUID.randomUUID().toString();
if (src.contains(tag))
continue;
if (dict.values().contains(tag))
continue;
if (dict.keySet().contains(tag))
continue;
break;
}

// Inject tag inside the dictionary
dict.put(key, tag);

// Recursive replace with a max depth of 10 levels.
String res;
for (int i = 0; i < 10; i++) {
// Do a replace with dictionary
res = StringReplacer.replaceFromMapping(src, dict);
if (res.equals(src))
break;
src = res;
}

// If the resulting string contains the tag, then the key is required
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not guaranteed: For example, of the string is "{foo}", a tag of "bar" is chosen (which is not in "{foo}"), but the "foo" variable contains "some string with bar", then this code will conclude that whatever key is passed is required, while it might not be.

Did you consider adding some instrumentation to replaceFromMapping to either check if a key is used, or more generically, let it return a list of keys it expanded?

Copy link
Member Author

Choose a reason for hiding this comment

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

This can't happen because tag is guaranteed to be different from any key/value contained in the map and either from any part of the pattern recipe, see how it is choosen in the while loop just before this.

Copy link
Member Author

@cmaglie cmaglie Nov 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Did you consider adding some instrumentation to replaceFromMapping to either check if a key is used, or more generically, let it return a list of keys it expanded?

This is trickier than it looks :-)

  • returning the keys it expanded, won't help in detecting if a key is required, it could be that, for example, {serial.port} is required but it's not expanded because the key serial.port it's not present in the preferences map.
  • we could "detect" a list of keys that needs to be replaced using a regexp that looks at matching braces in the recipes, but there are some recipes where the braces are actually needed! see this openocd command for instance:
    tools.openocd.program.pattern="{path}/{cmd}" {program.verbose} -s "{path}/share/openocd/scripts/" -f "{runtime.platform.path}/variants/{build.variant}/{build.openocdscript}" -c "telnet_port disabled; program {{{build.path}/{build.project_name}.elf}} verify reset; shutdown"
    the part {{{build.path}/{build.project_name}.elf}} will translate to {{my/sketch.elf}} but this doesn't mean that my/sketch.elf is a required field.

if (src.contains(tag)) {
throw new PreferencesMapException(key);
}
}

public static String[] formatAndSplit(String src, Map<String, String> dict) throws Exception {
String res;

Expand Down