plugins: add importhistory #4748

doronbehar · 2023-04-10T09:03:04Z

Description

Add a plugin useful for bulk importers.

To Do

Documentation. (If you've add a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
Tests. (Encouraged but not strictly required.)

JOJ0 · 2023-07-27T09:26:01Z

I have planned to finally look into this PR but I just can't get to it. In the meantime I'd like to mention that I have a quite similar plugin I'm maintaining (original maintainer doesn't use beets anymore). It certainly has a slightly different approach but just wanted to mention and maybe on the long run this could be merged into one plugin (....or not! It might be too different). Also it is pretty old code which I just quickfixed somehow to make it work for my needs. Just want you to have a look if you're interested: https://github.com/JOJ0/beets-dirfields

doronbehar · 2023-12-07T11:29:43Z

Hey @JOJ0 , after a mostly successful review process, I'd like to bump this PR upwards :) I hopefully fixed the formatting issues, and maybe CI will be green now. I also took a look upon the dirfields plugin and I think that this PR is much more mature - it includes docs as well as another feature described in the docs here, that isn't included in the competing plugin.

sampsyo

This is looking AWESOME! I really like the idea of a plugin dedicated to managing this information, and the use cases in the docs are super clear. Here are a few code suggestions!

docs/plugins/importhistory.rst

beetsplug/importhistory.py

JOJ0 · 2023-12-10T17:54:36Z

I finally found the time to read through the docs. Very useful and well written. Loads of useful usecases! Great!

Keeping track of the source path alone is the killerfeature for me❤️

I have one highlevel question regarding that: Will the sourcepath stay untouched in case for some reason I do a reimport from the items already in the library: beet import -L ...

sampsyo · 2023-12-10T18:29:21Z

Good question, @JOJ0!

doronbehar · 2023-12-13T21:26:03Z

Will the sourcepath stay untouched in case for some reason I do a reimport from the items already in the library: beet import -L ...

Good question, I had no idea this option existed. Is there a way to know what events this action triggers?

doronbehar · 2023-12-13T21:42:25Z

Thanks for all the comments. I'm glad you liked the idea in general, and that we only have to fine-tune the details of the implementation. I think that having tests would be nice, and it'll take me time to delve into it. I'll keep you notified! In the meantime, I pushed a commit fixing the easy issues presented by @sampsyo .

JOJ0 · 2023-12-14T22:33:53Z

Will the sourcepath stay untouched in case for some reason I do a reimport from the items already in the library: beet import -L ...

Good question, I had no idea this option existed. Is there a way to know what events this action triggers?

Best you just test a reimport using that option and see what value source_path has after that. I have a feeling that it might the beets library path and the original source_path gets lost. Give it a try and we'll see if it does. HTH :-)

doronbehar · 2024-03-14T22:57:30Z

Hey all,

I was busy for a while, and I had some time left today to fix some of the issues discussed back than, including a small bug I encountered while experimenting with the plugin personally. The commits I just pushed fix these issues, and I also marked the review conversations above as resolved, but of course feedback is still welcomed.

I didn't have time though, to think thoroughly about @JOJ0's question:

I have one highlevel question regarding that: Will the sourcepath stay untouched in case for some reason I do a reimport from the items already in the library: beet import -L ...

So don't feel like you need to review this now once more - this is still not ready enough.

doronbehar · 2024-03-15T16:17:12Z

Hmm now I checked and saw that (before the last push) using beet import --library on items that were imported, indeed changes the source_path field to the path from the music library. I managed to fix that by using an additional listener to the import_task_choice event.

doronbehar · 2024-03-15T16:24:46Z

Now I also fixed the small merge conflict, so this is ready for another round of review :). I think it'd be nice to write tests, but perhaps it'd be good to get your 👍 at this stage.

kergoth · 2024-09-20T17:56:37Z

I would suggest adding a config option to control the prompt to remove the source files, as I suspect there are those that would want to keep information about the import source but want to keep the source files pristine and unmodified otherwise. For example, I keep a copy of all the digital media I own for posterity, even if I remove it from my library, on general principle. Of course, this could always be added later on in a future PR.

Other than that, this looks like a great plugin, nice work.

JOJ0 · 2024-09-20T18:10:10Z

@kergoth I had a similar suggestion in mind...I think....

Do you mean a "global" config option for the plugin that disables all prompts ever and leaves the "source path saving" as the only feature left? Did I get this right? If not please clarify! Thanks! 😀

kergoth · 2024-09-20T18:48:55Z

@kergoth I had a similar suggestion in mind...I think....

Do you mean a "global" config option for the plugin that disables all prompts ever and leaves the "source path saving" as the only feature left? Did I get this right? If not please clarify! Thanks! 😀

Exactly what I was thinking, yeah.

doronbehar · 2024-09-22T15:12:37Z

Thanks for the comment :) I rebased to fix the merge conflicts and applied your suggestion. I also revised the documentation. Still I haven't found time yet to add a test for the plugin, and perhaps it'd be nice to do so.

JOJ0 · 2024-09-23T13:49:08Z

Hi @doronbehar thanks for the quick implementation of that feature! I'm currently reviewing and playing around with it so we can finally get to a merge. Yes, if you feel motivated a test for the plugin would be indeed wonderful!

I will push my suggestions directly as commits to your feature branch. Hope that is ok for you, we can discuss here then if you don't like something. It is just much easier for me at the moment to suggest as code/commits directly instead of adding suggestions via GitHub. Again: Hope that's ok, you'll see my commits soon......

PS: Last but not least: I want to apologize for letting you hang there for 6 months already! I was not really "in town" a couple of months ;-) Many thanks for the patience! Let's finally get this done now! 💪

JOJ0 · 2024-09-23T17:23:16Z

Hi @doronbehar I finished my changes for now. I tried to keep them bite-sized, so you can review them commit by commit. Hope that helps and speak soon. All the best!

JOJ0 · 2025-01-24T10:18:05Z

@doronbehar did you find the time to have a look at my changes? If you agree I'll do another round of review and I hope we can merge it soon. Thanks!

and keep one log message in one line.

Check the suggest_removal option value first thing. It does not make sense to move any further in this function

and also state item id.

instead of if/else.

instead of using the decode utf 8 built-in method. Wrap a couple more things with displayable_path that might have been overseen.

doronbehar · 2025-01-25T16:39:39Z

Oh I'm sorry @JOJ0 for not replying back then in September. I too was busy, and I'm still very patient with this so don't worry about this PR waiting for so long. I read your changes and I do approve them. The reason I'm holding this PR back a bit is the fact I think it is pretty important to add tests to it, and it seems a pretty hard task to endeavor at the moment. After encountering a hard to diagnose and explain bug with my personal usage of the plugin, I concluded this to be pretty important.

I won't mind if we'd convert this to a draft so it won't draw your attention as a task TODO.

sampsyo added the review-needed label May 21, 2023

doronbehar force-pushed the importhistory-plug branch 4 times, most recently from afafefb to 26e5ae7 Compare December 7, 2023 11:28

sampsyo reviewed Dec 10, 2023

View reviewed changes

doronbehar force-pushed the importhistory-plug branch 2 times, most recently from 79271b2 to e2f3ff8 Compare December 13, 2023 21:45

doronbehar force-pushed the importhistory-plug branch from c39d244 to 6ba5120 Compare March 15, 2024 16:16

doronbehar force-pushed the importhistory-plug branch from 6ba5120 to 1135b73 Compare March 15, 2024 16:20

doronbehar requested review from sampsyo and JOJ0 March 15, 2024 16:25

doronbehar force-pushed the importhistory-plug branch from bfeb828 to c475e96 Compare September 22, 2024 15:09

JOJ0 force-pushed the importhistory-plug branch from d05224b to 2ac8441 Compare September 23, 2024 17:17

doronbehar and others added 13 commits January 24, 2025 11:18

plugins: add importhistory

2e10077

importhistory: Fix deprecated logger syntax

20a6acd

importhistory: Shorten log msgs in suggest_removal

0df7d2c

and keep one log message in one line.

importhistory: Check suggest_removal option first

a6f1ecb

Check the suggest_removal option value first thing. It does not make sense to move any further in this function

importhistory: Shorten reimport info log msg

0debf28

and also state item id.

importhistory: Reimport check early continue

29f22e6

instead of if/else.

importhistory: Use displayable_path utility

ecf1ce1

instead of using the decode utf 8 built-in method. Wrap a couple more things with displayable_path that might have been overseen.

importhistory: Shorten comment about removal suggesting

18224ce

importhistory: Shorten comment about reimport event firing

386232d

importhistory: Shorten comment reimport overwrite

2f4d0dc

importhistory: Fix config option docs

d052a50

importhistory: Rewrite docs intro text

6ae73bb

importhistory: Config option docs format fix

165cd80

JOJ0 force-pushed the importhistory-plug branch from 0ca49dc to 165cd80 Compare January 24, 2025 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plugins: add importhistory #4748

plugins: add importhistory #4748

doronbehar commented Apr 10, 2023

JOJ0 commented Jul 27, 2023 •

edited

Loading

doronbehar commented Dec 7, 2023

sampsyo left a comment

JOJ0 commented Dec 10, 2023 •

edited

Loading

sampsyo commented Dec 10, 2023

doronbehar commented Dec 13, 2023

doronbehar commented Dec 13, 2023

JOJ0 commented Dec 14, 2023

doronbehar commented Mar 14, 2024

doronbehar commented Mar 15, 2024

doronbehar commented Mar 15, 2024

kergoth commented Sep 20, 2024

JOJ0 commented Sep 20, 2024 •

edited

Loading

kergoth commented Sep 20, 2024

doronbehar commented Sep 22, 2024

JOJ0 commented Sep 23, 2024 •

edited

Loading

JOJ0 commented Sep 23, 2024

JOJ0 commented Jan 24, 2025

doronbehar commented Jan 25, 2025

plugins: add importhistory #4748

Are you sure you want to change the base?

plugins: add importhistory #4748

Conversation

doronbehar commented Apr 10, 2023

Description

To Do

JOJ0 commented Jul 27, 2023 • edited Loading

doronbehar commented Dec 7, 2023

sampsyo left a comment

Choose a reason for hiding this comment

JOJ0 commented Dec 10, 2023 • edited Loading

sampsyo commented Dec 10, 2023

doronbehar commented Dec 13, 2023

doronbehar commented Dec 13, 2023

JOJ0 commented Dec 14, 2023

doronbehar commented Mar 14, 2024

doronbehar commented Mar 15, 2024

doronbehar commented Mar 15, 2024

kergoth commented Sep 20, 2024

JOJ0 commented Sep 20, 2024 • edited Loading

kergoth commented Sep 20, 2024

doronbehar commented Sep 22, 2024

JOJ0 commented Sep 23, 2024 • edited Loading

JOJ0 commented Sep 23, 2024

JOJ0 commented Jan 24, 2025

doronbehar commented Jan 25, 2025

JOJ0 commented Jul 27, 2023 •

edited

Loading

JOJ0 commented Dec 10, 2023 •

edited

Loading

JOJ0 commented Sep 20, 2024 •

edited

Loading

JOJ0 commented Sep 23, 2024 •

edited

Loading