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

(fix) Track Info: show key text as in tracks table #14020

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Dec 14, 2024

Fixes #14019 butit feels like this is just a workaround.
(I didn't look into the Track / TrackRecord details)

Seems like TrackRecord doesn't hold the key text from the database, whereas BaseTrackCache::getTrackValueForColumn() takes the Track's key text.

quoting my findings from #14011 (comment) again:
Max Graef - Jazz 104, FLAC

database ---
file metadata 8B
Tracks 8B
Deck 8B
Track Properties dialog ---
-> Re-import from file 8B

Based on #14011, so it's only the last commit.

@@ -744,7 +744,7 @@ mixxx::UpdateResult DlgTrackInfo::updateKeyText() {
}

void DlgTrackInfo::displayKeyText() {
const QString keyText = KeyUtils::keyToString(m_trackRecord.getKeys().getGlobalKey());
const QString keyText = m_pLoadedTrack ? m_pLoadedTrack->getKeyText() : QStringLiteral("");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I wonder if this is enough. In #14008 I found that getKeyText() could be empty and I had to use getKey().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, might also be getKeys().getGlobalKey().

I'm still digging a bit, trying to understand how the metadata flow is supposed to work.

@ronso0
Copy link
Member Author

ronso0 commented Dec 16, 2024

@m0dB so with #14008 being merged the preferred call for tracks table / track info consistency would be
KeyUtils::keyFromKeyTextAndIdValues(pTrack->getKeyText(), pTrack->getKey())
right?

@m0dB
Copy link
Contributor

m0dB commented Dec 16, 2024

Yes!

@ronso0
Copy link
Member Author

ronso0 commented Dec 16, 2024

Alright, done.

This will create conflicts for #14022, though since that is not urgent @daschuer assigned it to 2.5.1

@ronso0 ronso0 linked an issue Dec 17, 2024 that may be closed by this pull request
@ronso0 ronso0 added this to the 2.5.0 milestone Dec 17, 2024
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@daschuer daschuer merged commit 938cc31 into mixxxdj:2.5 Dec 17, 2024
13 checks passed
@ronso0 ronso0 deleted the keytext-trackinfo branch December 17, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

key empy in track Info, but displayed in tracks table
3 participants