Skip to content

Fix arduino101load argument passing if binary path contains spaces #419

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

Closed
wants to merge 1 commit into from

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Jan 31, 2017

This issue was first reported to the support channel and was solved by the proposed patch

@SidLeung
Copy link
Contributor

@calvinatintel @bigdinotech @eriknyquist , please review the change. @noelpaz @russmcinnis , please perform verification testing. Will merge the change to main trunk upon completion. Targeted for Deneb.

@SidLeung SidLeung added this to the Deneb milestone Jan 31, 2017
Copy link
Contributor

@calvinatintel calvinatintel left a comment

Choose a reason for hiding this comment

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

This is a fine solution, but I think {build.path} should be provided with the spaces escaped from the upstream.

@facchinm
Copy link
Member Author

http://forum.arduino.cc/index.php?topic=448542.0 shows the same problem indeed

@calvinatintel
Copy link
Contributor

@russmcinnis @noelpaz Please test

@novashah
Copy link

@russmcinnis @noelpaz Is this tested?

@noelpaz
Copy link
Contributor

noelpaz commented Mar 3, 2017

I tested this by saving the skecth in a folder with spaces in the name pleace note than when I named my Sketch "S P A C E D Blink", it renamed it into S_P_A_C_E_S__Blink and the IDE spit out this messgae:

The sketch name had to be modified. Sketch names can only consist
of ASCII characters and numbers (but cannot start with a number).
They should also be less than 64 characters long.

@calvinatintel explained that it is to verify that the enclosing folders that will have spaces. Fix is verified

Copy link
Contributor

@noelpaz noelpaz left a comment

Choose a reason for hiding this comment

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

Fix verified

@noelpaz noelpaz assigned facchinm and unassigned calvinatintel and noelpaz Mar 3, 2017
@noelpaz
Copy link
Contributor

noelpaz commented Mar 3, 2017

@facchinm -- fix verified. @yashaswini-hanji -- Please Squash and Merge

@noelpaz noelpaz assigned yashaswini-hanji and unassigned facchinm Mar 3, 2017
@yashaswini-hanji yashaswini-hanji removed their assignment Mar 3, 2017
@yashaswini-hanji
Copy link
Contributor

PR merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants