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.

Sign up for GitHub

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

add History cleanup options #4726

Merged
merged 10 commits into from
May 9, 2022
Merged

add History cleanup options #4726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
015674c
History cleanup: make track threshold configurable
ronso0 Apr 15, 2022
7b8a971
Preferences > Library: add spinbox for history cleanup track threshold
ronso0 Apr 15, 2022
a1c9d9a
History cleanup: option to keep locked playlists
ronso0 Apr 15, 2022
16f7565
move some [Library] ConfigKeys to library_prefs.cpp
ronso0 Apr 15, 2022
fbfc888
History: reset 'recently played' list when starting new session
ronso0 Apr 15, 2022
03ec672
History: make 'recently played' threshold configurable
ronso0 Apr 15, 2022
c59fc75
Pref > Library: re-group options
ronso0 Apr 16, 2022
a1ea8fc
PlaylistDAO: always keep locked playlists
ronso0 Apr 20, 2022
25e580a
History managemanent: use 'Track duplicate distance'
ronso0 Apr 23, 2022
2ec1ceb
History managemanent: use history_min_tracks_to_keep
ronso0 Apr 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
History managemanent: use 'Track duplicate distance'
  • Loading branch information
@ronso0
ronso0 committed Apr 23, 2022
commit 25e580ae8dce34fc57ae0aa01a3757f60f7b8a96
4 changes: 2 additions & 2 deletions src/library/library_prefs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ const ConfigKey mixxx::library::prefs::kHistoryCleanupMinTracksConfigKey =
mixxx::library::prefs::kConfigGroup,
QStringLiteral("history_cleanup_min_tracks")};
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the max length that is cleaned up?
how about something like: "max_tracks_auto_delete"
Or the other way around "min_entries_to_keep"

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, it's the "minimum tracks count to keep playlist":

"SELECT id FROM Playlists  "
"WHERE (SELECT count(playlist_id) FROM PlaylistTracks WHERE "
"Playlists.ID = PlaylistTracks.playlist_id) < :length AND "
"Playlists.hidden = :hidden AND Playlists.locked = :locked"

https://github.com/mixxxdj/mixxx/pull/4726/files#diff-4112e31df339e252ee2f3f49b18e1daf2ce58aaf488f5f4db43ab98c4aa83ff7

Copy link
Member

Choose a reason for hiding this comment

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

Right, for me this combination is sensible "min + keep" or "max + cleanup/delete" is correct.
"min + clenaup" feels wrong, while cleanup is not well defined.

From your explanation we may use "history_min_tracks_to_keep" or such.

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense, I changed it.
The preferences UI is okay IMO
image

Copy link
Member

Choose a reason for hiding this comment

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

would it be possible to substitute the N with the number widget inline?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you fear the N / X / ? placeholder will not be understood?

Puuting the actual number in the label would work but I don't see the benefit of this redundancy tbh.
Inline spinbox is also possible but its position depends on the translation, and it screws up the layout. Sooo: tricky..

Copy link
Member

Choose a reason for hiding this comment

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

Mhmm makes sense. I just thought it would result in better UX, but considering the challenges it causes, I agree that its not worth the effort.


const ConfigKey mixxx::library::prefs::kHistoryRecentlyPlayedThresholdConfigKey =
const ConfigKey mixxx::library::prefs::kHistoryTrackDuplicateDistanceConfigKey =
ConfigKey{
mixxx::library::prefs::kConfigGroup,
QStringLiteral("history_recently_played_threshold")};
QStringLiteral("history_track_duplicate_distance")};

const ConfigKey mixxx::library::prefs::kSearchDebouncingTimeoutMillisConfigKey =
ConfigKey{
Expand Down
4 changes: 2 additions & 2 deletions src/library/library_prefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ extern const ConfigKey kEditMetadataSelectedClickConfigKey;

extern const ConfigKey kHistoryCleanupMinTracksConfigKey;

extern const ConfigKey kHistoryRecentlyPlayedThresholdConfigKey;
extern const ConfigKey kHistoryTrackDuplicateDistanceConfigKey;

const int kHistoryRecentlyPlayedThresholdDefault = 6;
const int kHistoryTrackDuplicateDistanceDefault = 6;

const bool kEditMetadataSelectedClickDefault = false;

Expand Down
4 changes: 2 additions & 2 deletions src/library/trackset/setlogfeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) {

// Keep a window of 6 tracks (inspired by 2 decks, 4 samplers)
const unsigned int recentTrackWindow = m_pConfig->getValue(
kHistoryRecentlyPlayedThresholdConfigKey,
kHistoryRecentlyPlayedThresholdDefault);
kHistoryTrackDuplicateDistanceConfigKey,
kHistoryTrackDuplicateDistanceDefault);
while (m_recentTracks.size() > recentTrackWindow) {
m_recentTracks.pop_back();
}
Expand Down
14 changes: 7 additions & 7 deletions src/preferences/dialog/dlgpreflibrary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ void DlgPrefLibrary::initializeDirList() {

void DlgPrefLibrary::slotResetToDefaults() {
checkBox_library_scan->setChecked(false);
spinbox_history_recently_played_threshold->setValue(
kHistoryRecentlyPlayedThresholdDefault);
spinbox_history_track_duplicate_distance->setValue(
kHistoryTrackDuplicateDistanceDefault);
spinbox_min_history_tracks->setValue(1);
checkBox_SyncTrackMetadata->setChecked(false);
checkBox_SeratoMetadataExport->setChecked(false);
Expand All @@ -217,9 +217,9 @@ void DlgPrefLibrary::slotUpdate() {
checkBox_library_scan->setChecked(m_pConfig->getValue(
kRescanOnStartupConfigKey, false));

spinbox_history_recently_played_threshold->setValue(m_pConfig->getValue(
kHistoryRecentlyPlayedThresholdConfigKey,
kHistoryRecentlyPlayedThresholdDefault));
spinbox_history_track_duplicate_distance->setValue(m_pConfig->getValue(
kHistoryTrackDuplicateDistanceConfigKey,
kHistoryTrackDuplicateDistanceDefault));
spinbox_min_history_tracks->setValue(m_pConfig->getValue(
kHistoryCleanupMinTracksConfigKey, 1));

Expand Down Expand Up @@ -397,8 +397,8 @@ void DlgPrefLibrary::slotApply() {
m_pConfig->set(kRescanOnStartupConfigKey,
ConfigValue((int)checkBox_library_scan->isChecked()));

m_pConfig->set(kHistoryRecentlyPlayedThresholdConfigKey,
ConfigValue(spinbox_history_recently_played_threshold->value()));
m_pConfig->set(kHistoryTrackDuplicateDistanceConfigKey,
ConfigValue(spinbox_history_track_duplicate_distance->value()));
m_pConfig->set(kHistoryCleanupMinTracksConfigKey,
ConfigValue(spinbox_min_history_tracks->value()));

Expand Down
8 changes: 4 additions & 4 deletions src/preferences/dialog/dlgpreflibrarydlg.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@
<layout class="QGridLayout" name="gridLayout_history">

<item row="1" column="0" colspan="2">
<widget class="QLabel" name="label_history_recently_played_threshold">
<widget class="QLabel" name="label_history_track_duplicate_distance">
<property name="text">
<string>'Recently played' threshold</string>
<string>Track duplicate distance</string>
</property>
<property name="alignment">
<set>Qt::AlignLeft|Qt::AlignVCenter</set>
Expand All @@ -229,7 +229,7 @@
</widget>
</item>
<item row="1" column="2">
<widget class="QSpinBox" name="spinbox_history_recently_played_threshold">
<widget class="QSpinBox" name="spinbox_history_track_duplicate_distance">
<property name="toolTip">
<string>When playing a track again log it to the session history only if more than N other tracks have been played in the meantime</string>
</property>
Expand Down Expand Up @@ -533,7 +533,7 @@
<tabstop>radioButton_dbclick_bottom</tabstop>
<tabstop>radioButton_dbclick_top</tabstop>
<tabstop>radioButton_dbclick_ignore</tabstop>
<tabstop>spinbox_history_recently_played_threshold</tabstop>
<tabstop>spinbox_history_track_duplicate_distance</tabstop>
<tabstop>spinbox_min_history_tracks</tabstop>
<tabstop>checkBox_use_relative_path</tabstop>
<tabstop>spinBoxRowHeight</tabstop>
Expand Down