Consider LXQt theme names case-insensitively

[tsujan]

lxqt-config may also need a small change, but this PR ignores the letter case in the value of lxqt.conf → General → theme.

Closes lxqt/lxqt-themes#69

[stefonarch]

Nut sure about this PR if it's needed. I tested

stef@archlinux:themes$ sudo mv Valendas/ valenDas
stef@archlinux:themes$ sudo mv valenDas/ Valendas

After first renaming ambiance theme was started as expected as the theme disappeared, after second renaming the order isn't alphabetically anymore:

That's a side effect I think.

[tsujan]

The PR isn't for lxqt/lxqt-themes#69; it just closes it.

Here, the main idea is that theme names "should" be case-insensitive. LXQt Appearance Configuration → LXQt Theme already capitalizes the names, implying that they aren't case-sensitive, while they are. That's an inconsistency.

[tsujan]

after second renaming the order isn't alphabetically anymore:

That's related to lxqt-config. The PR is for liblxqt.

[stefonarch]

That's an inconsistency.

Ok, I see.
GTM

[tsujan]

In short, case-insensitivity means that you can only have one "dark" theme. If you want another one, you could call it "dark1", but not "Dark" — if you call it "Dark", it'll override "dark" because Qt sorts strings in that way and this patch picks up the first one.

[yan12125]

Thanks, the code logic looks reasonable, and it works with kde-plasma.

[tsujan]

@yan12125
Thanks for the review!

[palinek]

I, for one, don't like such "windows filesystem" file matching. If theme names are capitalized in lxqt-config view, I would say it should be chnged there. Just my 2 cents.

[tsujan]

If theme names are capitalized in lxqt-config view...

That was just an example that I mentioned, not the reason for the PR.

[tsujan]

The main reason:

Distinguishing between "kde-plasma", "KDE-Plasma" and "KDE-plasma" can be confusing to users. When it can be avoided by a simple change, I see no reason why we shouldn't make that change.

[palinek]

Distinguishing between "kde-plasma", "KDE-Plasma" and "KDE-plasma" can be confusing to users.

We're the main provider of themes. If we consider theme names confusing, we would just not provide them with such names. This is everything what average Joe needs.
But this case insensitive search & filtering adds confusion to somebody who will play with the themes. As we can't anyhow forbid one to create theme directories with the "same" name (case insensitive manner) and then one will observe either hiding the pre-existing theme or not showing the freshly created one. And I would say this is something unexpected and can bring us issue reports.

[stefonarch]

As we can't anyhow forbid one to create theme directories with the "same" name

This is a point. When I try changes I copy them to ~/.local/share/lxqt/themes, in the case of the KDE-Plasma renaming it would not be possible anymore to have both versions to compare in lxqt-config-appearance.

[tsujan]

and then one will observe either hiding the pre-existing theme...

Global themes are always overridden by user themes with the same names. Now, there's no need to worry about letter cases.

I don't think we need to worry about probable reports. If someone wants to play with "THEtheme" after making "theTHEME" (why should he?!), he'll realize what has happened.

[palinek]

he'll realize what has happened.

No, IMO he won't realize it. He will just see one theme in the GUI to select and consider it a bug.

[palinek]

Sorry, missed the button...

[tsujan]

No, IMO he won't realize it.

Let's wait and see ;)

[yan12125]

No, IMO he won't realize it.

Let's wait and see ;)

Technically, no users will be affected unless this PR is merged and released. I have no strong opinion on how lxqt/lxqt-themes#69 should be addressed, but keeping it open is not good.

[tsujan]

Technically, no users will be affected unless this PR is merged and released

Good point. Let's affect users ;)

[yan12125]

lxqt-config may also need a small change

Noticed one small thing: with this PR and theme=kde-plasma (all lowercase) in ~/.config/lxqt/lxqt.conf, no theme is selected in lxqt-config-appearance -> LXQt Theme. I guess case-insensitive matching is also needed there.

[tsujan]

I guess case-insensitive matching is also needed there.

Thanks! That was what I suspected (and mentioned in the PR's comment).

[tsujan]

Done in lxqt/lxqt-config#847

[tsujan]

after second renaming the order isn't alphabetically anymore:

I missed this comment before. Actually, the order was a small problem from start: user themes came first. All themes will be ordered alphabetically after lxqt/lxqt-config#848 (which requires #309).

[stefonarch]

user themes came first.

I liked that much: when modifying an existing theme I used a symlink to lxqt-themes/themes/theme in ~/.local/share/applications/ and it was clear because it was on top. With this change there is no way to know if some theme is local or in the system.

[tsujan]

It was so only by chance. For example, if you made a user theme out of Ambiance, you'd have no way of telling if it was a user theme by looking at the list.

Anyway, the list never distinguished between theme locations.

[tsujan]

In addition, now we can think about adding a real feature: distinguishing user themes in the list, e.g., by adding " (User Theme)" to their names. Maybe later...

[tsujan]

Your comment also reminded me of another missing feature: opening the theme folder by double clicking it in the list or by adding a context menu. Again, later...

[stefonarch]

Sounds good, even if it was only by chance it was a good feature and having distinctions again in a more clearer way is fine.
What about a division with titles: first user themes, then system themes or the other way round?

[tsujan]

What about a division with titles: first user themes

IMO, a list like this should be ordered alphabetically; otherwise, it'll be confusing. If the goal is distinguishing user themes, it can be achieved cleanly, without damaging UX and without extra options.