feat: add batch move with formatting of folder names #826

kiike · 2024-04-17T20:40:03Z

The other day I was adding changes to mv, and I realised I never used it since I didn't understand how it was supposed to work. Also, compared to a file manager, maybe a command that can only handle one document at a time is very underpowered.

So, today I wanted to change it more or less in line with what I did with rename.

This PR implements:

picking multiple folders
using --all like in other documents
format subfolders names
confirm before each move, and --batch to skip confirmation (what do you think: better to prompt at all? prompt before each move? or before executing the batch?)
prompting to add the changes if the library is version-controlled
documenting the changes for the docs
adding the info to the changelog (unless this feature is rejected, of course, let me know!)

Let me show you what it does:

before:

 papis -l test list -a
[INFO] database.cache: Indexing library. This might take a while...
/antoniou-2024-axialperturbationsofhairy
/dubinsky-2024-asymptoticdecayandquasinormal
/raju-2024-howdoesinformationemerge
/ray-2024-searchingforbinaryblack
/ressler-2024-blackhole-diskinteractionsin
/yuan-2024-primordialblackholeinterpretation

after running papis -l test mv -a --folder-name "{{doc.year}}":

 papis -l test list -a
[INFO] database.cache: Indexing library. This might take a while...
/2024/antoniou-2024-axialperturbationsofhairy
/2024/dubinsky-2024-asymptoticdecayandquasinormal
/2024/raju-2024-howdoesinformationemerge
/2024/ray-2024-searchingforbinaryblack
/2024/ressler-2024-blackhole-diskinteractionsin
/2024/yuan-2024-primordialblackholeinterpretation

Using slashes inside the format nests the subfolders, as expected:

 papis -l test mv -a --folder-name "{{doc.year}}/{{doc.tags[0]}}"
[... boring output ... ]
 papis -l test list -a
[INFO] database.cache: Indexing library. This might take a while...
/2024/to_read/antoniou-2024-axialperturbationsofhairy
/2024/to_read/dubinsky-2024-asymptoticdecayandquasinormal
/2024/to_read/raju-2024-howdoesinformationemerge
/2024/for_the_next_paper/ray-2024-searchingforbinaryblack
/2024/for_the_next_paper/ressler-2024-blackhole-diskinteractionsin
/2024/for_the_next_paper/yuan-2024-primordialblackholeinterpretation

As for the code, I tried to make it a bit more verbose, since these operations are quite delicate. So there are some more checks such as Document.get_main_folder() and the like.

I am reusing the --folder-name flag, although it's not defaulting to add-folder-name. Do you find it confusing?

I am trying a new small feature: if the library is under version control, I think it might be a good idea to prompt the user if they haven't supplied the --git option. If you think this might be a good idea, maybe it would be cool to also have an option in the config file to always add the changes or to never ask.

Tell me what you think about this PR! 😄

alexfikl · 2024-04-18T08:05:52Z

I've never used papis mv. What's the difference compared to papis rename?

kiike · 2024-04-18T08:16:17Z

Good question. From what I understood, the purpose of each is a bit different: you want mv when a document folder is not in a directory structure you like and so you want to organise that. You would want rename to change the folder name.

In other words, having this document:
MY_LIBRARY/foo/bar/RickAstley-2009-NeverGonna/info.yaml

rename would be used to act on RIckAstley-2009-NeverGonna
mv for foo/bar

The functionality overlaps a lot, I mean, it's just mv under the hood. I guess you could use rename to nest a document inside other folders and the move would work but I don't know how Papis would react. Why it was originally separated, I don't know 🤷. And whether having only mv for both operations is better or worse for UX, i don't know either. What do you think?

alexfikl · 2024-04-18T08:29:40Z

The functionality overlaps a lot, I mean, it's just mv under the hood. I guess you could use rename to nest a document inside other folders and the move would work but I don't know how Papis would react. Why it was originally separated, I don't know 🤷. And whether having only mv for both operations is better or worse for UX, i don't know either. What do you think?

Yeah, I don't really know why they were separate to begin with either. I'm mostly inclined to just have either papis rename or papis mv.. with a preference for papis mv to mirror how git names it too.

For similar semantics, there is a rename and a mv command on Linux:

mv: moves (potentially multiple) files or directories to somewhere.
rename: just renames a single file.

kiike · 2024-04-18T08:48:08Z

I agree. I would prefer just having a single mv too. And possibly make rename handle the attached files (so the files listed in files inside the info.yaml, as I think there is nothing that can let you rename a file after importing it). But I don't know how it would work UX-wise.

For instance, with the "git-like" semantics, ie. a single command to move and rename, imagine you have two docs, so "alex-2004-foo" and "alex-2004-bar", I guess if you do papis mv, pick the two documents, and choose "2004" as the dest folder, it's clear what you want. I mean, you wouldn't want to collapse the two directories into a single one. So you create the 2004 folder and move the two docs there. Easy.

But if you do papis mv, choose one document, and issue 2004 in the prompt, then it's not clear if you want to move it or rename. If you already have the 2004 folder, then I guess moving is what you want, mirroring what mv (posix program) does.

Just thinking out loud. How would you expect this all to work? I think we should invite other to the conversation.

alexfikl

This needs a bit more work and I left a few comments and suggestions. Probably worth leaving them alone until we decide what to do with rename vs mv though.

alexfikl · 2024-04-20T06:50:43Z

papis/commands/mv.py

+The ``mv`` command is used to organise a library moving their documents inside
+subfolders, which can be optionally created from a format adapted to each
+document.
+
+It will (except when run with ``--all``) bring up the picker with a list of
+documents that match the query. In the picker, you can select one or more
+documents and then initiate renaming by pressing enter. Folder names are cleaned,
+so that various characters (white spaces, punctuation, capital letters, and some
+others) are automatically converted.


Suggested change

The ``mv`` command is used to organise a library moving their documents inside

subfolders, which can be optionally created from a format adapted to each

document.

It will (except when run with ``--all``) bring up the picker with a list of

documents that match the query. In the picker, you can select one or more

documents and then initiate renaming by pressing enter. Folder names are cleaned,

so that various characters (white spaces, punctuation, capital letters, and some

others) are automatically converted.

The ``mv`` command is used to organise a library by moving the documents.

Unlike the ``rename`` command, this command can move documents to subfolders

and has richer formatting options. When used, it will (except when run with

``--all``) bring up the picker with a list of documents that match the query.

In the picker, you can select one or more documents and then initiate the move

by pressing enter. Folder names are cleaned, so that various characters (white

spaces, punctuation, capital letters, and some others) are automatically converted

(see :confval:`doc-paths-extra-chars`).

alexfikl · 2024-04-20T06:54:07Z

papis/commands/mv.py

+- You can use formatting rules to generate the folder name too. For instance, to
+  organise all documents by author "Rick Astley" by year, you can use (Python and
+  Jinja2 formatting, respectively):
+
+   .. code:: sh
+
+        # Python format
+        papis mv --folder-name "{doc[year]}" --all author:"Rick Astley"
+
+        # Jinja2 format
+        papis mv --folder-name "{{doc.year}}" --all author:"Rick Astley"


Suggested change

- You can use formatting rules to generate the folder name too. For instance, to

organise all documents by author "Rick Astley" by year, you can use (Python and

Jinja2 formatting, respectively):

.. code:: sh

# Python format

papis mv --folder-name "{doc[year]}" --all author:"Rick Astley"

# Jinja2 format

papis mv --folder-name "{{doc.year}}" --all author:"Rick Astley"

- You can use formatting rules to generate the folder name too. For instance, to

organise all documents by author "Rick Astley" by year, you can use:

.. code:: sh

papis mv --folder-name "{doc[year]}" --all author:"Rick Astley"

All the examples in the documentation use the Python formatter, because that's the default one. Since the formatter is essentially a plugin, it feels weird to just single out the python and jinja2 ones to give examples of here.

Fair enough! I'll use this change then!

alexfikl · 2024-04-20T06:56:32Z

papis/commands/mv.py

        raise DocumentFolderNotFound(papis.document.describe(document))

    if git:
-        papis.git.mv(folder, new_folder_path)
+        papis.git.mv(doc_main_folder, dest_path)


This should also commit? What do you think?

My thought is that if it just moves, then you'd need to call a separate papis git -p <QUERY> commit "Moved ?? from ?? to ??" or something like that.

I think if the file is tracked it should commit, yes.

papis/commands/mv.py

alexfikl · 2024-04-20T07:03:50Z

papis/commands/mv.py

+        if dest_path is None:
+            logger.error("Failed to construct a folder path for the document.")
+            return


Suggested change

if dest_path is None:

logger.error("Failed to construct a folder path for the document.")

return

if not dest_path:

logger.error("Failed to construct a folder path for the document: %s",

papis.document.describe(document))

continue

? To just skip the document?

I'm never sure how to handle this, e.g. the update command will exit right away if it fails to update some key in some document, but most other commands will skip it and continue.

alexfikl · 2024-04-20T07:05:26Z

papis/commands/mv.py

+        if folder_name is not None:
+            subfolder = papis.format.format(folder_name, document)
+            dest_path = os.path.join(lib_dir, subfolder)


If folder_name is None, shouldn't we just skip the whole thing? i.e. this check should be moved outside the loop and return.

Then here it should check if subfolder, i.e. if the formatting didn't end up making an empty string for whatever reason and warn about that.

alexfikl · 2024-04-20T07:12:40Z

papis/commands/mv.py

+    for document in documents:
+        # If the user has passed ``--folder-name``, use that to generate the subfolder
+        if folder_name is not None:
+            subfolder = papis.format.format(folder_name, document)


This not correct, since the document keys can contain additional path separators. There's similar logic in papis add that we should refactor and reuse here.

For example, if the title is Face/Off and the folder format is {doc[year]}/{doc[title]} we'll get unexpected results.

Yes! And also special characters. I'll see what add does and adapt for here.

I've tried to do some refactoring in #829 to extract that logic. Probably needs a bit more work, but it should be doable to use it here as well.

alexfikl · 2024-04-20T07:14:38Z

papis/commands/mv.py

+        if not os.path.exists(dest_path):
+            logger.info("Creating path '%s'.", dest_path)
+            os.makedirs(dest_path, mode=papis.config.getint("dir-umask") or 0o666)


Does this correctly check that we're not overwritting existing documents? I didn't see any checks in run() either.

I don't think it does! It should check and skip the move, like rename (IIRC) does.

alexfikl · 2024-04-20T07:21:28Z

I agree. I would prefer just having a single mv too. And possibly make rename handle the attached files (so the files listed in files inside the info.yaml, as I think there is nothing that can let you rename a file after importing it). But I don't know how it would work UX-wise.

I think papis update can handle that these days.. but it doesn't rename the files on disk (I think?), so it's not exactly the complete solution.

For instance, with the "git-like" semantics, ie. a single command to move and rename, imagine you have two docs, so "alex-2004-foo" and "alex-2004-bar", I guess if you do papis mv, pick the two documents, and choose "2004" as the dest folder, it's clear what you want. I mean, you wouldn't want to collapse the two directories into a single one. So you create the 2004 folder and move the two docs there. Easy.

But if you do papis mv, choose one document, and issue 2004 in the prompt, then it's not clear if you want to move it or rename. If you already have the 2004 folder, then I guess moving is what you want, mirroring what mv (posix program) does.

Just thinking out loud. How would you expect this all to work? I think we should invite other to the conversation.

Maybe this can work similarly to how papis add works, which has both a --subfolder and a --folder-name option. Then --subfolder will always force a subfolder to be created, but --folder-name can also be subfolder/my-doc-folder if the subfolder already exists.

Would that make sense? Not sure if the behavior of papis add is the best, but at least they would work similarly. Do you think it makes more sense to have separate commands for the two cases?

kiike · 2024-04-20T14:05:49Z

Thanks you so much for the review!

I think papis update can handle that these days.. but it doesn't rename the files on disk (I think?), so it's not exactly the complete solution.

Yes, it looks like it doesn't handle file renames, additions or deletions, as per the TODO comment there. You can change the files list but it won't do anything to the files. Plus you have to pass Python-style syntax, but with all the shell quoting needed.

Maybe this can work similarly to how papis add works, which has both a --subfolder and a --folder-name option. Then --subfolder will always force a subfolder to be created, but --folder-name can also be subfolder/my-doc-folder if the subfolder already exists.
Would that make sense? Not sure if the behavior of papis add is the best, but at least they would work similarly. Do you think it makes more sense to have separate commands for the two cases?

To be honest, I don't know what would be better for a user. I would be perfectly comfortable having just mv that renames as needed. It would simplify the code and also less mental overhead (otherwise you want to (1) rename a folder and then (2) change the folder structure). But I don't know if that would be just me.

alexfikl · 2024-04-20T16:35:45Z

Plus you have to pass Python-style syntax, but with all the shell quoting needed.

I'm not sure I follow, what does "Python-style syntax" mean here? (I haven't actually used papis update to rename things, so I'm mostly just curious)

To be honest, I don't know what would be better for a user. I would be perfectly comfortable having just mv that renames as needed. It would simplify the code and also less mental overhead (otherwise you want to (1) rename a folder and then (2) change the folder structure). But I don't know if that would be just me.

Hm, I don't think I was clear here, since it sounds to me like I agree with what you're saying.

What I was suggesting was a way to resolve some ambiguity that you mentioned (?) when doing a
papis mv --folder-name "subfolder" <QUERY>: it's not clear if you want to move the documents to that subfolder or rename the picked document.

In that case, I would suggest adding a --subfolder flag (mostly because papis add has one) so that:

If you want to just rename folders or move them to some existing subfolder you can use --folder-name.
If you want to move them to a subfolder, you can force that with --subfolder.

I think that's similar to how the -t/--target-directory flag works in the Unix mv command. Does that make sense?

kiike · 2024-04-20T16:44:21Z

Plus you have to pass Python-style syntax, but with all the shell quoting needed.

I'm not sure I follow, what does "Python-style syntax" mean here? (I haven't actually used papis update to rename things, so I'm mostly just curious)

I was just rambling about papis update. If you want to change the files of a document, you need to pass something like --set files '["file1.pdf", "file2.pdf"]' or something like that. That's what I meant with "python-style syntax".

To be honest, I don't know what would be better for a user. I would be perfectly comfortable having just mv that renames as needed. It would simplify the code and also less mental overhead (otherwise you want to (1) rename a folder and then (2) change the folder structure). But I don't know if that would be just me.

Hm, I don't think I was clear here, since it sounds to me like I agree with what you're saying.

What I was suggesting was a way to resolve some ambiguity that you mentioned (?) when doing a papis mv --folder-name "subfolder" <QUERY>: it's not clear if you want to move the documents to that subfolder or rename the picked document.

In that case, I would suggest adding a --subfolder flag (mostly because papis add has one) so that:
* If you want to just rename folders or move them to some existing subfolder you can use `--folder-name`.

* If you want to move them to a subfolder, you can force that with `--subfolder`.
I think that's similar to how the -t/--target-directory flag works in the Unix mv command. Does that make sense?

Yep! Having subfolder and folder-name sounds like something that removes a lot of the ambiguity. Would you like me to try to make this change that unifies both commands and aliases rename to mv? I would do this after the documentation, since I think the release might come first.

alexfikl · 2024-04-20T16:50:33Z

I was just rambling about papis update. If you want to change the files of a document, you need to pass something like --set files '["file1.pdf", "file2.pdf"]' or something like that. That's what I meant with "python-style syntax".

Ah, yeah, I didn't quite like that either: the command-line string is quite clunky (like in your example) and then you also need to do an eval on it..

Suggestions are very welcome!

Yep! Having subfolder and folder-name sounds like something that removes a lot of the ambiguity. Would you like me to try to make this change that unifies both commands and aliases rename to mv? I would do this after the documentation, since I think the release might come first.

Yeah, that sounds good, we can definitely merge this with whatever fixes you have time to make from my comments above. It's already a pretty big improvement on the previous state of papis mv! 🎉

jghauser · 2024-04-23T20:54:51Z

I finally managed to have a look at the conversation here!

In other words, having this document: MY_LIBRARY/foo/bar/RickAstley-2009-NeverGonna/info.yaml
* `rename` would be used to act on `RIckAstley-2009-NeverGonna`

* `mv` for `foo/bar`

In general I think there is sense in having a distinction between mv and rename because they are different in the minds of everyone except very geeky people 😃. Rename means changing the file/folder name only and move means putting the file/folder into a different directory and I think that's roughly what the commands do (though I don't actually use either of them). So I'm ok with keeping both (with rename mostly being a wrapper around mv). It's a bit like papis tag is just a more convenient version of papis update. On the other hand, I do see the purpose of keeping the sub-command list short, so maybe getting rid of rename isn't so bad (also, for some reason, when I hear 'rename', I think of the attached files and not the folder)...

I think papis update can handle that these days.. but it doesn't rename the files on disk (I think?), so it's not exactly the complete solution.

Yes, it looks like it doesn't handle file renames, additions or deletions, as per the TODO comment there. You can change the files list but it won't do anything to the files. Plus you have to pass Python-style syntax, but with all the shell quoting needed.

Yeah, I don't think we need papis rename to handle files if there is papis update whose functionality could be extended (after all, you're gonna always want to rename the files in concert with the info in the .yaml).

Btw, you don't need to use papis update --set WEIRDPYTHONSTRING. You can use papis update -p files FILE1 -p files FILE2 if you want to add two files (maybe with a papis update --drop files if you want to clear the list first) or papis update --rename files OLDFILENAME NEWFILENAME if you want to rename something. I mostly added the python string stuff in case someone wants to add e.g. a dictionary or something else that is otherwise not supported. If there's any core functionality that requires python syntax, we (I 😅) should fix that. Maybe I should also add a note to the docs that this functionality is 'advanced'.

alexfikl · 2024-04-24T06:52:35Z

So I'm ok with keeping both (with rename mostly being a wrapper around mv). It's a bit like papis tag is just a more convenient version of papis update.

That's a good point! I'm also ok with just keeping both as long as the code is heavily shared. (Don't need to do it as part of this PR, but something to keep in mind)

Btw, you don't need to use papis update --set WEIRDPYTHONSTRING. You can use papis update -p files FILE1 -p files FILE2 if you want to add two files (maybe with a papis update --drop files if you want to clear the list first) or papis update --rename files OLDFILENAME NEWFILENAME if you want to rename something. I mostly added the python string stuff in case someone wants to add e.g. a dictionary or something else that is otherwise not supported. If there's any core functionality that requires python syntax, we (I 😅) should fix that.

Ha! I didn't realize that you can just use -p files file1 -p files file2! That's a lot nicer!

Maybe I should also add a note to the docs that this functionality is 'advanced'.

I would definitely appreciate making that clearer in the docs 😁

kiike force-pushed the feat/batch_move branch from d4ecc20 to 10a768d Compare April 17, 2024 20:46

feat: add batch move with formatting of folder names

534fd81

kiike force-pushed the feat/batch_move branch from 10a768d to 534fd81 Compare April 17, 2024 21:06

alexfikl requested changes Apr 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add batch move with formatting of folder names #826

feat: add batch move with formatting of folder names #826

kiike commented Apr 17, 2024 •

edited

Loading

alexfikl commented Apr 18, 2024

kiike commented Apr 18, 2024 •

edited

Loading

alexfikl commented Apr 18, 2024

kiike commented Apr 18, 2024

alexfikl left a comment

alexfikl Apr 20, 2024

alexfikl Apr 20, 2024

kiike Apr 20, 2024

alexfikl Apr 20, 2024

kiike Apr 20, 2024

alexfikl Apr 20, 2024

alexfikl Apr 20, 2024

alexfikl Apr 20, 2024

kiike Apr 20, 2024

alexfikl Apr 20, 2024

alexfikl Apr 20, 2024

kiike Apr 20, 2024

alexfikl commented Apr 20, 2024

kiike commented Apr 20, 2024

alexfikl commented Apr 20, 2024 •

edited

Loading

kiike commented Apr 20, 2024

alexfikl commented Apr 20, 2024

jghauser commented Apr 23, 2024

alexfikl commented Apr 24, 2024

feat: add batch move with formatting of folder names #826

Are you sure you want to change the base?

feat: add batch move with formatting of folder names #826

Conversation

kiike commented Apr 17, 2024 • edited Loading

alexfikl commented Apr 18, 2024

kiike commented Apr 18, 2024 • edited Loading

alexfikl commented Apr 18, 2024

kiike commented Apr 18, 2024

alexfikl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfikl commented Apr 20, 2024

kiike commented Apr 20, 2024

alexfikl commented Apr 20, 2024 • edited Loading

kiike commented Apr 20, 2024

alexfikl commented Apr 20, 2024

jghauser commented Apr 23, 2024

alexfikl commented Apr 24, 2024

kiike commented Apr 17, 2024 •

edited

Loading

kiike commented Apr 18, 2024 •

edited

Loading

alexfikl commented Apr 20, 2024 •

edited

Loading