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

ftintitle: New customization option to keep feature in artist field #5356

Merged
merged 8 commits into from
Sep 6, 2024

Conversation

klb2
Copy link
Contributor

@klb2 klb2 commented Jul 10, 2024

Description

This PR adds a new option keep_in_artist to the ftintitle plugin.

Previous Behavior

The artist field is always changed to match the albumartist field.

New Behavior

The new option allows a customization of the old behavior. If the option is set to yes/true, the "feat. X" part is kept in the artist field.
In order to be backwards compatible and not introduce unexpected behavior, the default is set to false, i.e., matching the old behavior.

Benefits

  • Additional customization
  • Searching for features using both title and artist (this can be very helpful when using music players with GUI)

To Do

  • Documentation.
  • Changelog.
  • Tests.

klb2 added 5 commits July 9, 2024 23:49
The new keep_in_artist option allows keeping the feat. part in the
artist metadata field while still changing the title.
Add functionality tests for the new keep_in_artist option of the
ftintitle plugin.
@klb2 klb2 changed the title Ftintitle keep artist ftintitle: New customization option to keep feature in artist field Jul 10, 2024
Copy link
Member

@JOJ0 JOJ0 left a comment

Choose a reason for hiding this comment

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

This looks very straight forward. Thanks! I think it doesn't hurt if you also add the typical usecase you mention in the PR description to the docs as well:

Searching for features using both title and artist (this can be very helpful when using music players with GUI)

Either in the docs introductional text or in the option description itself.

Depending on where you put it, you might want to rephrase. Again just brainstorming here. Something like:

....To be able to search for features in artist fields as well, this option can be used.....

It's helpful in music players that let us search in artist fields in general, not only with GUI.

beetsplug/ftintitle.py Outdated Show resolved Hide resolved
klb2 and others added 3 commits September 5, 2024 12:15
Update the documentation of the new `keep_in_artist` option, adding an
example use case.
Update the log message when the artist is kept unchanged due to setting
the keep_in_artist option to true.
Copy link
Contributor Author

@klb2 klb2 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Please let me know if you have other change requests

@klb2
Copy link
Contributor Author

klb2 commented Sep 5, 2024

I have updated the documentation and log message as you have suggested. Please let me know if there are any other changes you would like to see.

@klb2 klb2 requested a review from JOJ0 September 5, 2024 10:35
Copy link
Member

@JOJ0 JOJ0 left a comment

Choose a reason for hiding this comment

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

Many thanks. This is good to go 🎉

beetsplug/ftintitle.py Show resolved Hide resolved
docs/plugins/ftintitle.rst Show resolved Hide resolved
@JOJ0 JOJ0 merged commit 98f4a88 into beetbox:master Sep 6, 2024
12 checks passed
@klb2 klb2 deleted the ftintitle_keep_artist branch September 9, 2024 09:25
@chrishoage
Copy link

This PR produces errors for me

Traceback (most recent call last):
  File "/lsiopy/bin/beet", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/lsiopy/lib/python3.12/site-packages/beets/ui/__init__.py", line 1865, in main
    _raw_main(args)
  File "/lsiopy/lib/python3.12/site-packages/beets/ui/__init__.py", line 1852, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/lsiopy/lib/python3.12/site-packages/beets/ui/commands.py", line 1400, in import_func
    import_files(lib, paths, query)
  File "/lsiopy/lib/python3.12/site-packages/beets/ui/commands.py", line 1331, in import_files
    session.run()
  File "/lsiopy/lib/python3.12/site-packages/beets/importer.py", line 360, in run
    pl.run_parallel(QUEUE_SIZE)
  File "/lsiopy/lib/python3.12/site-packages/beets/util/pipeline.py", line 447, in run_parallel
    raise exc_info[1].with_traceback(exc_info[2])
  File "/lsiopy/lib/python3.12/site-packages/beets/util/pipeline.py", line 312, in run
    out = self.coro.send(msg)
          ^^^^^^^^^^^^^^^^^^^
  File "/lsiopy/lib/python3.12/site-packages/beets/util/pipeline.py", line 195, in coro
    func(*(args + (task,)))
  File "/lsiopy/lib/python3.12/site-packages/beets/importer.py", line 1668, in plugin_stage
    func(session, task)
  File "/lsiopy/lib/python3.12/site-packages/beets/plugins.py", line 143, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/lsiopy/lib/python3.12/site-packages/beetsplug/ftintitle.py", line 122, in imported
    self.ft_in_title(item, drop_feat)
TypeError: FtInTitlePlugin.ft_in_title() missing 1 required positional argument: 'keep_in_artist_field'

Line here

self.ft_in_title(item, drop_feat)

I am able to reproduce just by running beets import -q /path/to/music

@klb2
Copy link
Contributor Author

klb2 commented Sep 21, 2024

Thank you for spotting this @chrishoage. I have created a PR to fix this (#5433)

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