Skip to content
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

Add the ability to provide remote destination filename or keep relative path in destination filename #5

Merged
merged 4 commits into from
Mar 5, 2016

Conversation

loicortola
Copy link
Contributor

First, let me thank you for this awesome tool that you have provided.
I use it a lot and I think it is a great replacement for the python tool.

For many use-cases (especially webservers and static files), it is important for an upload script to keep the paths in the filenames.

As you know, NodeMCU's filesystem is key/value based and it is possible to have slashes in a filename.

The default behaviour for this was to strip the path, and only keep the filename.

I propose this PR which gives the user two more options in the CLI: --keeppath and --remotename.

N.B: This changes the upload function signature a little. If you have important concerts about backward compatibility, thanks for letting me know, and I will rearrange the code to trick it.

@AndiDittrich
Copy link
Owner

Dear loicortola,

thank you for your contribtion! its a great addon - i will check the code and give you a feedback within the next week.

best reagrds, Andi

@loicortola
Copy link
Contributor Author

Thanks, looking forward to it.

@AndiDittrich
Copy link
Owner

In lib/NodeMcuConnector.js line 21

compile: 'node.comgpile("?")'

is this a typo or a new NodeMCU core function ?

@loicortola
Copy link
Contributor Author

Haha! Haven't seen that One. Must have been between the tests and the commit. I will change this right away

AndiDittrich pushed a commit that referenced this pull request Mar 5, 2016
Code Review - Add the ability to provide remote destination filename or keep relative path in destination filename
@AndiDittrich AndiDittrich merged commit 4f194f4 into AndiDittrich:master Mar 5, 2016
@AndiDittrich
Copy link
Owner

Thanks! i've just merged your pull request. a release will follow the next days after sucessful testing :)

@AndiDittrich
Copy link
Owner

Dear loicortola,

i've found a possible issue with the keeppath option: when having multiple projects and using relative paths these paths are transfered with their relative placeholders.

Example 1

./nodemcu-tool upload ../NodeMCU-Toolkit/wifi.lua --keeppath
[NodeMCU-Tool] Connected
[NodeMCU] Version: 0.9.5 | ChipID: 0xd1aa | FlashID: 0x1640e0
[NodeMCU-Tool] Uploading "../NodeMCU-Toolkit/wifi.lua" ...
[NodeMCU-Tool] Data Transfer complete!

./nodemcu-tool fsinfo
[NodeMCU-Tool] Connected
[NodeMCU] Version: 0.9.5 | ChipID: 0xd1aa | FlashID: 0x1640e0
[NodeMCU] Free Disk Space: 3343 KB | Total: 3346 KB | 1 Files
[NodeMCU] Files stored into Flash (SPIFFS)
          |- ../NodeMCU-Toolkit/wifi.lua (2151 Bytes)

Current Solution

I have added a simple filter which removes the relative parts - is this change in your interest ?

// filename defaults to original filename minus path.
// this behaviour can be overridden by --keeppath and --remotename options
var remoteFile = options.remotename ? options.remotename : (options.keeppath ? localFile : _path.basename(localFile));

// normalize the remote filename (strip relative parts)
remoteFile = remoteFile.replace(/\.\.\//g, '').replace(/\.\./g, '').replace(/^\.\//, '');

// display filenames
logStatus('NodeMCU-Tool', 'Uploading "' + localFile + '" >> "' + remoteFile + '"...');

Result

./nodemcu-tool upload ../NodeMCU-Toolkit/wifi.lua --keeppath
[NodeMCU-Tool] Connected
[NodeMCU] Version: 0.9.5 | ChipID: 0xd1aa | FlashID: 0x1640e0
[NodeMCU-Tool] Uploading "../NodeMCU-Toolkit/wifi.lua" ...
NodeMCU-Toolkit/wifi.lua
[NodeMCU-Tool] Data Transfer complete!

./nodemcu-tool fsinfo
[NodeMCU-Tool] Connected
[NodeMCU] Version: 0.9.5 | ChipID: 0xd1aa | FlashID: 0x1640e0
[NodeMCU] Free Disk Space: 3341 KB | Total: 3346 KB | 2 Files
[NodeMCU] Files stored into Flash (SPIFFS)
          |- NodeMCU-Toolkit/wifi.lua (2151 Bytes)

best regards, Andi

@loicortola
Copy link
Contributor Author

Hi Andi,
you are right, that may have posed a problem.
Indeed, this change makes sense and it doesn't break anything on my side either, so perfect :)

Thanks for the input, best wishes,

Loïc

@AndiDittrich
Copy link
Owner

Great. I've published the v1.4 release :)

@AndiDittrich AndiDittrich added this to the v1.4 milestone Mar 12, 2016
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.

2 participants