Use naming mask in download paths

marklieberman / downloadstar

Download all items in a webpage that match a pattern

GNU General Public License v3.0

90 stars 16 forks source link

Use naming mask in download paths #63

Closed tigregalis closed 5 years ago

tigregalis commented 5 years ago

Partially addresses https://github.com/marklieberman/downloadstar/issues/52

Addresses https://github.com/marklieberman/downloadstar/issues/62

I've reused the logic for naming masks and applied it to folder paths, so that you can name a subfolder (and nested subfolders below that) based on variables.

Preview: downloadstar-folder-masks

Later, I added frameTitle and tabTitle as variables for the naming mask.

There was also an issue with one of the sass imports, when I npm installed locally, the filename was solid rather than fa-solid.

I'm still learning, so open to all feedback.

marklieberman commented 5 years ago

PR looks very nice. I will test and merge after New Years. Thank you.

tigregalis commented 5 years ago

No problem. I realised that with the tab title, if there are slashes in the title (like on this page), there is probably a need to replace those slashes, otherwise you end up nesting folders untentionally. I've tentatively updated scrape.js, but perhaps it should be handled further down the chain.

A couple other things:

There is also an issue when any part/segment of a folderpath or filename begins or ends with a period ., so that exception probably needs to be handled, and the user informed. Actually that's mentioned in https://github.com/marklieberman/downloadstar/issues/47
The user interface probably should have a folderpath preview, like the filename preview.

marklieberman commented 5 years ago

As you say, sanitation of the title should probably happen as late as possible. That can be done as part of the fix for #58. I also need to add some features to the UI so I will look into adding a folder path preview at that time. Also I need to add the variables frameTitle and tabTitle you added need to the wiki article.

marklieberman commented 5 years ago

After fixing the premature sanitization (#58), I feel it is easier and more intuitive to accomplish the same task just by having slashes in the naming mask. For example: ${frameTitle}/${fileext}. As you stated, it is possible to accidentally nest if there are slashes in the variables. So, lets use it as a feature instead. Furthermore, it doesn't require me to add validation to the download path input box or any additional previewing logic for the final path.

I will also add a filter for convenience that strips slashes and is equivalent to |replace:\/:_, called |noFolder. (e.g.: ${frameTitle|noFolder}/${fileext})

tigregalis commented 5 years ago

Nice work. When do you see the next release?

marklieberman commented 5 years ago

A new release (2.0.3) is up on AMO. A change landed in Nightly that broke the addon, so I had to do an update.