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

rewrite: Clarify the scope of the wide-ranging effects of the plugin, which can sometimes include modifying actual metadata #2786

Closed
fdaniele85 opened this issue Jan 8, 2018 · 11 comments · Fixed by #4668
Labels

Comments

@fdaniele85
Copy link
Contributor

fdaniele85 commented Jan 8, 2018

Hi guys. In the documentation of rewrite plugin it is written that "Note that this plugin only applies to templating; it does not modify files' metadata tags or the values tracked by beets' library database.", but it seems that it modifies the tags.

Without rewrite plugin I import an album by "Buckethead & Friends" and I obtain

beet ls enter the chicken --format '$albumartist - $title'
Buckethead & Friends - Intro
Buckethead & Friends - Nottingham Lace
Buckethead & Friends - Coma
Buckethead & Friends - Waiting Hare
Buckethead & Friends - The Hand
Buckethead & Friends - Funbus
Buckethead & Friends - Botnus
Buckethead & Friends - Running From the Light
Buckethead & Friends - We Are One
Buckethead & Friends - Three Fingers
Buckethead & Friends - Interlude

But enabling the rewrite plugin with this config file:

import:
    move: yes

directory: /media/Esterno/Organizzati

plugins: inline convert	rewrite

rewrite:
    albumartist .*buckethead.*: Buckethead

paths:
    default: $albumartist/$original_year - $album%aunique{}/%if{$multidisc,Disc $disc/}$track - $title
    singleton: $albumartist/$original_year - $album%aunique{}/%if{$multidisc,Disc $disc/}$track - $title
    comp: $albumartist/$original_year - $album%aunique{}/%if{$multidisc,Disc $disc/}$track - $title

item_fields:
    multidisc: 1 if disctotal > 1 else 0

convert:
    formats:
        mp3: ffmpeg -i $source -y -vn -aq 0 $dest

I obtain

beet ls enter the chicken --format '$albumartist - $title'
Buckethead - Intro
Buckethead - Nottingham Lace
Buckethead - Coma
Buckethead - Waiting Hare
Buckethead - The Hand
Buckethead - Funbus
Buckethead - Botnus
Buckethead - Running From the Light
Buckethead - We Are One
Buckethead - Three Fingers
Buckethead - Interlude

And inspecting the tags with easytag, the albumartist is simply Buckethead. I would like to maintain the correct tags, but change the saving path.

Thanks.

@wisp3rwind
Copy link
Member

I took a quick glance at the plugin code, and I believe that the documentation is rather misleading. The way it seems to work, it does indeed not modify the database initially (not entirely sure about the tags).

However the full scope of the rewriting is not well controlled. For example, the database entry will be rewritten if the field is read (this retrieves the modified version) and then stored back without further changes. I guess that would happen when using the edit plugin even when the field is not actually changed, but diplayed in the file. For the same reason, beet ls also shows the rewritten version even if the database has not been changed.

I'm not sure how to deal with this (I also don't use the plugin, so I'm not exactly familiar with its usecases), but the current behaviour does not appear very sensible to me, given the analysis above is correct (which I did not test). I guess the plugin should really somehow hook into the template formatting process, and not modify field retrieval in general.

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Jan 10, 2018
@sampsyo
Copy link
Member

sampsyo commented Jan 10, 2018

Indeed! That's a good summary of the issues, @wisp3rwind. The plugin doesn't just rewrite fields for paths; it rewrites them for all field lookups in beets. That is a pretty big hammer.

If anyone's interested in digging deeper and finding a more limited way to accomplish something similar, I'd be happy to help out.

@fdaniele85
Copy link
Contributor Author

fdaniele85 commented Jan 14, 2018

I'm not a Python expert, but I wrote a simple plug-in to manage this situation. Probably can be done better, but this is the code I'm using at the moment:

from beets.plugins import BeetsPlugin
import re

_substitute_rules = []

class Substitute(BeetsPlugin):
    def __init__(self):
        super(Substitute, self).__init__()
        self.template_funcs['substitute'] = _tmpl_substitute

        for key, view in self.config.items():
            value = view.as_str()
            pattern = re.compile(key.lower())
            _substitute_rules.append((pattern, value))

def _tmpl_substitute(text):
    if text:
         for pattern, replacement in _substitute_rules:
              if pattern.match(text.lower()):
                  return replacement
         return text
    else:
         return u''

in the config file:

paths:
  default: %substitute{$albumartist}/$year - $album%aunique{}/%if{$multidisc,Disc $disc/}$track - $title
...
substitute:
    pattern: substitution string

Maybe something similar can be added to default plugins...

@sampsyo
Copy link
Member

sampsyo commented Jan 14, 2018

That’s a cool idea! It’s a nice, compact plugin that would fit in as a rewrite alternative. We could totally consider shipping it with beets.

@stale
Copy link

stale bot commented Jul 12, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 12, 2020
@stale stale bot closed this as completed Jul 19, 2020
@Maxr1998
Copy link
Contributor

It seems that not a lot has changed about this. The documentation is still misleading, and while I like how the rewrite plugin works right now for most use-cases, it would be good to have an alternative plugin that does what's specified in the docs.

For example, I have recently changed my replace config to not replace trailing dot characters with underscores anymore since that isn't necessary on my Linux system.
However, I'd want to keep the location for previously imported artists/artists unchanged because my music player doesn't like file location changes (it resets import dates and breaks playlists).
Thus, I thought about just doing an artist R.E.M.: R.E.M_ in the rewrite section; however, I was pretty surprised that this also changed the tags in the files. Not exactly what I wanted here.

Shipping something like the above plugin would be handy in this case.

@zhelezov
Copy link
Contributor

Why is this one closed? Situation with plugin behaviour/docs is unchanged.

@sampsyo
Copy link
Member

sampsyo commented Jan 25, 2023

The main reason is that we didn't settle concretely on a path forward. Maybe we should make this specifically about fixing the docs and reopen it?

@Maxr1998
Copy link
Contributor

I think that would be a good idea. What about the substitute plugin that was suggested above? Do you think that would make sense to have a PR opened for it, if @fdaniele85 agrees?

@fdaniele85
Copy link
Contributor Author

I think that would be a good idea. What about the substitute plugin that was suggested above? Do you think that would make sense to have a PR opened for it, if @fdaniele85 agrees?

I agree

@sampsyo sampsyo reopened this Jan 26, 2023
@stale stale bot removed the stale label Jan 26, 2023
@sampsyo sampsyo added docs and removed needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." labels Jan 26, 2023
@sampsyo sampsyo changed the title Rewrite plugin modifies tags rewrite: Clarify the scope of the wide-ranging effects of the plugin, which can sometimes include modifying actual metadata Jan 26, 2023
@zhelezov
Copy link
Contributor

zhelezov commented Feb 7, 2023

I think that would be a good idea. What about the substitute plugin that was suggested above? Do you think that would make sense to have a PR opened for it, if @fdaniele85 agrees?

I agree

That would be great, we will have a plugin that does what is advertised finally 😀

@fdaniele85 fdaniele85 mentioned this issue Feb 7, 2023
3 tasks
JOJ0 added a commit to fdaniele85/beets that referenced this issue Aug 31, 2023
in the plugin docs as was discovered in beetbox#2786 (Metadata being modified).
@JOJ0 JOJ0 closed this as completed in #4668 Sep 8, 2023
Louson pushed a commit to Louson/beets that referenced this issue Sep 26, 2023
in the plugin docs as was discovered in beetbox#2786 (Metadata being modified).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants