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

Optional more verbose duplicates prompt #4866

Merged
merged 5 commits into from
Aug 23, 2023
Merged

Conversation

JOJ0
Copy link
Member

@JOJ0 JOJ0 commented Jul 31, 2023

Description

A new config option allows to enable a more detailed view when a duplicate is detected during import of an album or singleton.

  • New import config option:
import:
    duplicate_verbose_prompt: no
  • Disabled by default.

To Do

  • Documentation.
  • Changelog.
  • Tests.

Comment on lines 876 to 878
if not task.is_album:
print(f" {duplicate}")
else:
for dup in duplicate.items():
print(f" {dup}")
Copy link
Member

Choose a reason for hiding this comment

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

This might be slightly easier to read if it were reversed! (That is, start with if task.is_album:.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hahaha, I had it the other way round and thought: That looks badly readable and switched it around. But actually I agree with you, starting with a negated condition when there is an else branch anyway, is often confusing. I'll change it back! :-)

JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 1, 2023
JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 1, 2023
JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 1, 2023
@JOJ0 JOJ0 marked this pull request as ready for review August 1, 2023 11:27
@arsaboo
Copy link
Contributor

arsaboo commented Aug 1, 2023

Thanks @JOJ0

It would be nice to get this related PR to handle upgrades merged soon.

JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 2, 2023
JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 2, 2023
JOJ0 added a commit to JOJ0/beets that referenced this pull request Aug 4, 2023
@JOJ0
Copy link
Member Author

JOJ0 commented Aug 5, 2023

Alright, I'm quite happy with this feature now and it's ready for a final review. Some nitpicking on my docs-wording and format might be good! Thanks in advance!

Hmm, I left out the fact that also singleton imports would now present a more verbose prompt in my docs text. Is that bad? Should I add it?

Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

Nice! No objections here—looks pretty solid to me.

@JOJ0 JOJ0 merged commit 717f379 into beetbox:master Aug 23, 2023
14 checks passed
Louson pushed a commit to Louson/beets that referenced this pull request Sep 26, 2023
Louson pushed a commit to Louson/beets that referenced this pull request Sep 26, 2023
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.

3 participants