search

JacobMcAuley / playlist_import

A module for Foundry VTT for importing playlists.
GNU General Public License v3.0
7 stars 13 forks source link

Naming patch #27

Closed Ariphaos closed 3 years ago

Ariphaos commented 3 years ago

Tries to more intelligently name tracks and playlists. Converts CamelCase, underscores to spaces, etc.

JacobMcAuley commented 3 years ago

Hi @Ariphaos!

Thanks for the help and thank you for your patience!

I've made the mistake of not being thorough enough when merging patches, such that I accidentally introduce new issues. To help prevent that, would you mind providing some example test cases so that I may use your provided tests as a baseline?

Additionally, did you test your naming patch on 0.7.9? From my, albeit brief, research .replaceAll() seems to be throwing errors. I've attempted a short test on my end and found similar results. Though, it is completely possible this is a user error and I'm a fool. Worst case and there is an error, you can easily remedy it!

Cleaning up the naming convention is something long neglected and I'm happy you took the up torch for it! I can't wait to merge it! :smile:

Examples of some shorthand test cases I'd like: "this_song_is_neat.mp3" --> "this song is neat" camelCaseTest --> camelCaseTestResult

Ariphaos commented 3 years ago

Hrm, what browser are you testing on? I didn't get any errors in Firefox. Or more specifically what errors are you getting?

I've done this to import more than a thousand sounds at this point. >_>

"this_song_is_neat.ogg" -> "This Song is Neat" "camelCaseTest.mp3" -> "Camel Case Test" "123_CherryRoad.mp3" -> "Cherry Road" "DownToTheBakery.ogg" -> "Down to the Bakery"

JacobMcAuley commented 3 years ago

Oh, hopefully I didn't come across wrong. I believe you and I'm sure it works, but the doozy is that if something goes wrong I get the DMs on discord asking to fix it. I just want to make sure I everything goes swimmingly.

As for the browser, I use the standalone executable and I'm testing on Foundry 0.7.9. The error is as reported, but here is a screen shot just to elaborate a bit more on the specifics. image

Ariphaos commented 3 years ago

Oh, it looks like replaceAll is non-standard. And probably unnecessary. I'll test a bit further and edit the PR.

Ariphaos commented 3 years ago

There. This also addresses #28

JacobMcAuley commented 3 years ago

Wonderful! Thank you. As for #28, I believe that to be a vestigial remainder from when Foundry's permission system was slightly less verbose. It never even crossed my mind to update it, so thanks for handling that. In the future, I may adjust the naming to allow for custom additions to small = ['a', 'an', 'at', 'and', 'but', 'by', 'for', 'if', 'nor', 'on', 'of', 'or', 'so', 'the', 'to', 'yet']; mostly so that languages other than English can benefit from it (considering your work is amazing it would be a shame not to :smile: )

I've updated your name to the list of contributors on the README.