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

Move command should support copying files without modifying database #435

Closed
fortes opened this issue Oct 22, 2013 · 28 comments
Closed

Move command should support copying files without modifying database #435

fortes opened this issue Oct 22, 2013 · 28 comments
Labels
feature features we would like to implement good first issue https://github.com/beetbox/beets/pull/5479

Comments

@fortes
Copy link
Member

fortes commented Oct 22, 2013

Currently the copy flag on the move command will copy files and change their paths in the database. It would be useful to have a version of this that copies without modifying the library.

@sampsyo
Copy link
Member

sampsyo commented Oct 22, 2013

Good idea. Maybe the flag should be called -e for --external or --export?

@fortes
Copy link
Member Author

fortes commented Oct 22, 2013

Export makes sense. Could also be a separate command instead of just a flag. I think either will work.

@sampsyo sampsyo added the good first issue https://github.com/beetbox/beets/pull/5479 label Jan 26, 2016
@ghost
Copy link

ghost commented Jan 29, 2016

Hey guys! we would like to tackle this issue, if anyone is going to work on this soon let me know so we can look at other issues! We're planning to start working on this on Monday February 1st. Thanks! -Jake

@SpirosChadoulos
Copy link
Contributor

Greetings..I was thinking about tackling this issue but it is really old. Is it worth tackling it or not? Thanks a lot.

@sampsyo
Copy link
Member

sampsyo commented Apr 8, 2017

Absolutely, if you're interested!

@SpirosChadoulos
Copy link
Contributor

Nice, could you explain me what you want me to do exactly?

@sampsyo
Copy link
Member

sampsyo commented Apr 8, 2017

The idea would be to add a flag to the move command, with one of the names above, that works like the current -c (copy) option but slightly different. Where the current option does two things—it copies the files and also updates the library database—the new -e option would instead copy the file but leave the database unchanged. I think that will involve a call along the lines of util.copy(item.path, item.destination(basedir=...)).

@SpirosChadoulos
Copy link
Contributor

Should I do that in the commnad.py file or somewhere else?

@sampsyo
Copy link
Member

sampsyo commented Apr 9, 2017

Right in commands.py is the right place.

@SpirosChadoulos
Copy link
Contributor

Could you explain to me briefly how the move_items function works in commands.py(line1453)?
Thanks a lot.

@sampsyo
Copy link
Member

sampsyo commented Apr 9, 2017

I'd be happy to explain more, but can I please ask you to be specific with your question? The comments we've put there are already my best effort at explaining what's going on. What part specifically are you wondering about?

@SpirosChadoulos
Copy link
Contributor

SpirosChadoulos commented Apr 9, 2017

I see that the move_items function is where the move command is executed but I can't distinguish where the database interaction is happening.

@sampsyo
Copy link
Member

sampsyo commented Apr 9, 2017

Aha! Good question. That's the obj.remove call, which invokes either Item.remove or Album.remove, which are defined in library.py.

@SpirosChadoulos
Copy link
Contributor

In line 1491 in command.py obj.move is called. obj.remove is called at line 1217. I am kind of confused with the code that is executed when the user uses the move command.

@sampsyo
Copy link
Member

sampsyo commented Apr 10, 2017

Oops! I'm so sorry; I lost track of what we were talking about and looked at the wrong function (delete_items). Anyway, you're right—the call to obj.move is the one you're looking for. That both moves the file and updates the stored path to reflect the new location.

@SpirosChadoulos
Copy link
Contributor

So should I make changes to the move_items function in commands.py or in the move function in the Item class in library.py?

@sampsyo
Copy link
Member

sampsyo commented Apr 10, 2017

Good question! Just in the move_items function there. Let's leave the core library functionality unchanged—we can just change the way we invoke it.

@SpirosChadoulos
Copy link
Contributor

I think that the move command is specifically happening at line 1491:obj.move(copy, basedir=dest). If this is true how am I supposed to make it so that the database path doesn't change?

@sampsyo
Copy link
Member

sampsyo commented Apr 11, 2017

There's no current way to make item.move not update the database—updating the database is its purpose in life—but I suggested an alternative solution above in #435 (comment). Alternatively, if you think it would be easier to just add a simple change to library.py, we can of course discuss that too.

@SpirosChadoulos
Copy link
Contributor

What do you mean by adding a flag to the move command?

@sampsyo
Copy link
Member

sampsyo commented Apr 12, 2017

I just mean adding one more command-line option, like these ones:
https://github.com/beetbox/beets/blob/master/beets/ui/commands.py#L1506-L1524

@SpirosChadoulos
Copy link
Contributor

Something like: move_cmd.parser.add_option( u'-e', u'--export', default=False, action='store_true', help=u'...' ) ?? And then where do I write the main code of the feature??

@sampsyo
Copy link
Member

sampsyo commented Apr 12, 2017

Yep, like that! Then, you'll notice that the options get exposed in the opts argument to the command function, and then those values get passed to the move_items function. That last function is where the actual work will go.

@SpirosChadoulos
Copy link
Contributor

What do you mean by exposed?

@sampsyo
Copy link
Member

sampsyo commented Apr 12, 2017

The values are available—for example, as opts.pretend and opts.force here: https://github.com/beetbox/beets/blob/master/beets/ui/commands.py#L1562

@SpirosChadoulos
Copy link
Contributor

So the work that will happen in the move_items function should be in an if statement like if export ??

@sampsyo
Copy link
Member

sampsyo commented Apr 12, 2017

Yep, exactly!

@SpirosChadoulos
Copy link
Contributor

Okay I will start the work and stay in touch for any further explanation. Thanks a lot!

sampsyo added a commit that referenced this issue Jun 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement good first issue https://github.com/beetbox/beets/pull/5479
Projects
None yet
Development

No branches or pull requests

3 participants