NickvisionApps / Tagger

Tag your music
https://flathub.org/apps/details/org.nickvision.tagger
MIT License
211 stars 13 forks source link

Fixes + Tweaks #396

Closed nlogozzo closed 8 months ago

nlogozzo commented 8 months ago

Fixes #395

TODO:

WinUI Tweaks:

nlogozzo commented 8 months ago

@kissthermite Could you test this build: https://github.com/NickvisionApps/Tagger/actions/runs/6776895782

kissthermite commented 8 months ago

@kissthermite Could you test this build: https://github.com/NickvisionApps/Tagger/actions/runs/6776895782

sure! will let you know in a moments

kissthermite commented 8 months ago

Using custom format string /%albumartist%/%album%/%artist% - %title% with Limit File Name Characters setting turn on does work as expected for others limited characters except /


tag to file name conversion with aforementioned format string with limited character being :

will give this output: /Kotono Nagase (CV_Mirai Tachibana)/song for you(Kotono version)/Kotono Nagase (CV_Mirai Tachibana) - song for you(Kotono version).flac


tag to file name conversion with aforementioned format string with limited character being /

will give this output: /SQUARE ENIX MUSIC/NieR Gestalt & NieR Replicant Original Soundtrack/SQUARE ENIX MUSIC - Song of the Ancients /Devola.flac


nlogozzo commented 8 months ago

tag to file name conversion with aforementioned format string with limited character being /

Well now with the new algorithm / means new directory. But you want it where if the / was contained in a tag property and now the format string, to not count as a new directory, correct?

I think I can do that.

kissthermite commented 8 months ago

But you want it where if the / was contained in a tag property and now the format string, to not count as a new directory, correct?

Correct indeed.

nlogozzo commented 8 months ago

@fsobolev If you could make sure the algorithm in this commit still holds: https://github.com/NickvisionApps/Tagger/pull/396/commits/325b0f408260cd0d5a69dfc756fda2e213346a14

nlogozzo commented 8 months ago

But you want it where if the / was contained in a tag property and now the format string, to not count as a new directory, correct?

Correct indeed.

@kissthermite Try this build: https://github.com/NickvisionApps/Tagger/actions/runs/6779510126?pr=396

P.S. Thanks for testing and sorry for messing up your library structure with these tests 🫠

nlogozzo commented 8 months ago

But you want it where if the / was contained in a tag property and now the format string, to not count as a new directory, correct?

Correct indeed.

@kissthermite Try this build: https://github.com/NickvisionApps/Tagger/actions/runs/6779510126?pr=396

P.S. Thanks for testing and sorry for messing up your library structure with these tests 🫠

Sorry I lied, please try this build: https://github.com/NickvisionApps/Tagger/actions/runs/6779656223

If a tag property contains /, regardless of using Limit Filename Characters, it will not create a new directory. Only if the format string explicitly contains a /, then it will.

kissthermite commented 8 months ago

P.S. Thanks for testing and sorry for messing up your library structure with these tests 🫠

Don't worry my actual music library are stored on /var/home/kissthermite/Jellyfin Server Media/Music The one on Downloads folder are just a copy for testing purpose XD.

kissthermite commented 8 months ago

If a tag property contains /, regardless of using Limit Filename Characters, it will not create a new directory. Only if the format string explicitly contains a /, then it will.

Just tested and indeed it does behave the way you said whether Limit File Name Characters turn on/off. Good job @nlogozzo! and enjoy your well deserved rest tonight.

nlogozzo commented 8 months ago

Just tested and indeed it does behave the way you said whether Limit File Name Characters turn on/off. Good job @nlogozzo! and enjoy your well deserved rest tonight.

Yay, glad it's working as expected now! Once @fsobolev approves of the rest of this PR, I will release a beta.

If you have any other issues/feature requests please try to report them before Wednesday that way I can implement them all on Wednesday and then get a stable out by Friday 😅

kissthermite commented 8 months ago

If you have any other issues/feature requests please try to report them before Wednesday that way I can implement them all on Wednesday and then get a stable out by Friday 😅

Actually there's one thing. With Limit File Name Characters turn on, Tagger ignored space before, between & after that limited characters, is that the intended behavior?

nlogozzo commented 8 months ago

With Limit File Name Characters turn on, Tagger ignored space before, between & after that limited characters, is that the intended behavior?

Hmm...no it's not intended. Will take a look at it tomorrow to see what's going on.

nlogozzo commented 8 months ago

With Limit File Name Characters turn on, Tagger ignored space before, between & after that limited characters, is that the intended behavior?

@kissthermite Can't reproduce... Here's WinUI: image Here's GNOME: image

I think it just seems every small in GNOME but the space is indeed there.

kissthermite commented 8 months ago

Yeah. My bad just check it again and indeed it's just a very tiny space.

nlogozzo commented 8 months ago

I was going to release a beta but decided against it since it's almost Friday and if any bugs come up before then I will fix them and just release stable. Since your issue was confirmed to be fixed, no need for a beta.