NickvisionApps / Tagger

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

On Linux if `File Name` & `Title` contain `<` character, Tagger `File Name` & `Title` preview will show nothing #370

Closed kissthermite closed 10 months ago

kissthermite commented 10 months ago

1

2

Not that related but i think it's better to force rename File Name if forbidden printable ASCII characters of Windows (< > : " / \ | ? *) are present to _ even on Linux version of Tagger for compatibility reason.

nlogozzo commented 10 months ago

The issue with this wasn't the fact of it being an invalid file name character. The problem was gtk/adw widgets by default support a markup language by default when setting text and < trigger that markup. So we needed to set use-markup to false and that allows the row to display correctly:

image

nlogozzo commented 10 months ago

Not that related but i think it's better to force rename File Name if forbidden printable ASCII characters of Windows (< > : / \ | ? *) are present to _ even on Linux version of Tagger for compatibility reason.

Tagger already checks when changing the file name if the character is valid or not on that system. For Linux, the only invalid character is /. So if you try to change the file name to something with /, the / will be replaced by _. On the Windows version of Tagger, it checks and replaces those invalid characters.

As far as replacing invalid Windows characters on Linux too, I don't think we should do that since most Linux users won't be using Windows and since no other Linux program checks if file names are Windows compatible (besides Parabolic if the option is turned on lol, so maybe we can add that here too?) @fsobolev thoughts?

kissthermite commented 10 months ago

As far as replacing invalid Windows characters on Linux too, I don't think we should do that since most Linux users won't be using Windows and since no other Linux program checks if file names are Windows compatible (besides Parabolic if the option is turned on lol, so maybe we can add that here too?) @fsobolev thoughts?

Understandable, i suppose cross platfrom audio filename incompatibility are outside of Tagger scope.

nlogozzo commented 10 months ago

outside of Tagger scope.

Like I said, Parabolic has this preference: image

So we could definitely add that to Tagger too

kissthermite commented 10 months ago

Like I said, Parabolic has this preference: image

So we could definitely add that to Tagger too

Hell yeah! that would be awesome. Since Android also have the same filename limitation as Windows.

nlogozzo commented 10 months ago

Use https://github.com/NickvisionApps/Tagger/issues/372 to track

nlogozzo commented 10 months ago

Please test the latest beta release with this fix: https://github.com/NickvisionApps/Tagger/releases/tag/2023.10.0-beta2 and let us know how it goes!

Will be available via flathub-beta within the hour

kissthermite commented 10 months ago

Please test the latest beta release with this fix: https://github.com/NickvisionApps/Tagger/releases/tag/2023.10.0-beta2 and let us know how it goes!

name

< character no longer causing Tagger preview to behave weirdly. my thought? My satisfaction is immeasurable and my day is saved