psyinfra / onyo

text-based inventory system on top of git
ISC License
3 stars 5 forks source link

replace `onyo rm -a/-d` with better `onyo rm --recursive` #596

Open TobiasKadelka opened 4 months ago

TobiasKadelka commented 4 months ago
❱ onyo rm --help                                                                                                            130 !
usage: onyo rm [-h] [-a] [-d] [-m MESSAGE] PATH [PATH ...]

Delete ASSETs and/or DIRECTORYs.

Directories and asset directories are deleted along with their contents.

The `--asset` and `--dir` flags can be used to constrain actions to
either assets or directories (respectively).

If any of the given paths are invalid, Onyo will error and delete none of
them.

positional arguments:
  PATH                  Assets and/or directories to delete.

options:
  -h, --help            show this help message and exit
  -a, --asset           Operate only on assets. Asset Files are removed. Asset Directories
                        are converted into normal directories.

                        This cannot be used with the `--dir` flag.
  -d, --dir             Operate only on directories. Directories are removed. Asset
                        Directories are converted into Asset Files.

                        This cannot be used with the `--asset` flag.
  -m MESSAGE, --message MESSAGE
                        Use the given MESSAGE as the commit message (rather than the default).
                        If multiple `-m` options are given, their values are concatenated as
                        separate paragraphs.

I find the formulations of --dir and --asset could be a bit more clear. "Operate only on assets. Asset Files are removed. Asset Directories are converted into normal directories." and "Operate only on directories. Directories are removed. Asset Directories are converted into Asset Files." are of course technically true statements, but I am convinced that the "Operate only ..." part will confuse every single new onyo user (while advanced users probably wont read it) and I think that phrase should either simply be omitted or be "Assets and asset dirs".

It is also missleading in so far that "Operate only on dirs" feels weird to me as a first sentence because it sounds like this is the main effect of this flag -- but instead, if literally changes how onyo rm opperates on asset dirs. Instead of deleting them, it just undoes the asset dir aspect. Why I find this explanation very problematic becomes obvious with this example (using onyo rm --dir to delete a dir and an asset dir in one call):

❱ onyo rm --dir ethics/Achilles\ Book/laptop_microsoft_surface.oq782j shelf 
The following will be deleted:
-/Users/tkadelka/onyo/ethics/Achilles Book/laptop_microsoft_surface.oq782j
-/Users/tkadelka/onyo/shelf/None_None_None.None
-/Users/tkadelka/onyo/shelf

I know that some people will argue "asset dirs are assets" and "asset dirs are directories", and I will argue that explicite help text for commands like this will help new users to understand the "onyo mindset".

TobiasKadelka commented 4 months ago

It was decided in a meeting to remove both flags and instead require a flag onyo rm --recursive to delete folders with contents.