Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skins/Shade: Remove unused qss declarations #2374

Merged
merged 1 commit into from
Dec 2, 2019

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Nov 28, 2019

Found with my qsscheck.py script:

$ ./qsscheck.py .
./res/skins/LateNight/style.qss:741:5: ParseError - expected ':'
./res/skins/LateNight/style.qss:1198:3: ParseError - expected ':'
./res/skins/Shade/style.qss:9:1: Unknown object name "#Deck"
./res/skins/Shade/style.qss:248:3: Unknown object name "#LibraryTimesPlayed"

Manual verification by grepping through the code did not yield any results, so it seems that this is not a false positive.

The parse errors in LateNight are already fixed in PR #2342.

@ronso0
Copy link
Member

ronso0 commented Nov 28, 2019

(this is not relevant for Shade but)
I remember removing selectors like #Deck1 because they weren't used explicitely but composed like Deck<Variable name="chanNum"/> so we have to pay attention, or even consider such cases in the script.

@Holzhaus
Copy link
Member Author

Holzhaus commented Nov 28, 2019

Yes, the script already does this. It regex-replaces <.*> with * in the object names that were found, then does a glob-like match against the selector that we're currently looking at.

I also verified the results via grep:

$ grep -Hrn 'Deck<' res/skins/Shade
$ grep -Hrn 'setObjectName("Deck' src
$ grep -Hrn 'LibraryTimesPlayed' src
$ grep -Hrn 'LibraryTimesPlayed' res/skins/Shade
res/skins/Shade/style.qss:248:  #LibraryTimesPlayed::item {
$

@Holzhaus
Copy link
Member Author

Holzhaus commented Dec 2, 2019

I didn't notice any difference when running Mixxx with the Shade skin so I think the qsscheck output isn't a false positive. Merge?

@daschuer
Copy link
Member

daschuer commented Dec 2, 2019

I also don't see any differences. LGTM Thank you.

@daschuer daschuer merged commit 4b38536 into mixxxdj:master Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants