[Feature]: Update subcommands to be more consistent

[MikaMika]

Problem

mpqcli --help | grep file | cut -d' ' -f3 | # any subcommand that
while read -r subcommand                    # interacts with files
do mpqcli $subcommand --help | grep 'mpqcli'
done

mpqcli create [OPTIONS] target
mpqcli add [OPTIONS] archive files...
mpqcli remove [OPTIONS] archive files...
mpqcli list [OPTIONS] target
mpqcli extract [OPTIONS] target
mpqcli read [OPTIONS] file target

Sometimes files are after archive,
sometimes before,
sometimes as option,
sometimes multiple,
sometimes only one.

Proposed solution

archive files... is my favorite.

Lots of these can be solved with more knowledge of CLI:

• create only accepts 1 file
  • would be mpqcli create [OPTIONS] <dir | file [files...]>
  • But you can create a temp dir and place multiple files inside
• list will print all files
  • would be mpqcli read [OPTIONS] <archive> [pattern]
  • But you can grep it
• extract requires --file option at most 1
  • would be mpqcli extract [OPTIONS] <archive> [files...]
• read is the opposite order of add and remove, making it single file only
  • would be mpqcli read [OPTIONS] <archive> <file> [files...]
  • How would multiple files read? Like cat *.txt or like grep . *.txt?

Additionally, convert all / to \
Example: mpqcli extract archive.mpq scripts/*.txt sounds/surfthe.wav
But maybe I'm wrong on this point, as you can get the filenames with list and pipe grep xargs and stuff.

Additional context

No response

[thomaslaurenson]

I am glad you took the time to report this issue. And thanks for providing all the examples, it really helps to see the current state outlined - and it highlights why the current implementation is not intuitive. It should be intuitive! And it should follow the same logical structure.

The original design was roughly:

mpqcli [OPTIONS] [FILE|DIRECTORY] target

The target was always last - which made sense, because MPQ archives were the target (focus) of the library. But then more functionality was added over time. We discussed command structure lots in PRs and issues, as there are so many ways to implement something. I guess you know this already, because a lot of discussion was in issues you created.

Long story short: I agree completely. This needs to be sorted and cleaned up.

I have been thinking about this for a while, because I knew the design sucked. Going to write up a first stage of a re-design plan in my next comment.

[thomaslaurenson]

Proposed Rules

1. Change naming conventions for positional arguments
   1. Change target to archive when referencing MPQ archives so this is clearer
   2. Change all instances of input arguments (e.g., local files, directories, and files in an mpq archive) to "file" or "files"
   3. There is no difference for a positional argument when using "file" or "files" when the argument refers to a file on disk, or a filepath in the MPQ archive. This is how pretty much all other unix-like archiving tools are designed. The subcommand verb (e.g., add) should indicate if the file is a local file or a file path in the MPQ archive
2. Some subcommands that only take archive as a positional argument have no change to positional arguments. They have one positional argument: archive. These subcommands are:
   1. info
   2. list
   3. verify
   4. compact
3. Some subcomands are designed to only take only one file as input. These subcommands are:
   1. rename: This is special. It is a 1:1 operation which should take: rename archive old-file new-file. Does not support input from stdin.
   2. read: Also special. I think this should stay a single file input and should be: read archive file. However, this command should accept input from stdin.
4. Remaining subcommands that can take multiple file input: create, add, remove, extract
   1. Should be changed to follow this general structure: subcommand [OPTIONS] archive <files>
   2. One exception... for extract the files argument is option, no argument means extract all
   3. These subcommands should all support multiple files (either local files on disk or filepaths inside the MPQ
   4. These subcommand should also all support stdin

I know this is a long summary. I needed to iterate and write a full set of rules for me to work through the problem and find a solution.

Proposed positional arguments

Subcommand	Current	Proposed	Multiple files	Stdin	Change
info	info <target>	info <archive>	-	-	Rename only
list	list <target>	list <archive>	-	-	Rename only
verify	verify <target>	verify <archive>	-	-	Rename only
compact	compact <target>	compact <archive>	-	-	Rename only
add	add <archive> <files>	add <archive> <files>	Yes	Yes	None
remove	remove <archive> <files>	remove <archive> <files>	Yes	Yes	None
rename	rename <archive> <old-file> <new-file>	rename <archive> <old-file> <new-file>	No	No	None
create	create <target>	create <archive> <files>	Yes	Yes	Archive becomes positional
extract	extract <target>	extract <archive> [files]	Yes	Yes	Files become positional
read	read <file> <target>	read <archive> <file>	No	Yes	Order reversed

Notes:

• <files> required
• [files] optional
• version, about and completion take no positional arguments and are unaffected

Summary - Three Types of Commands

mpqcli <subcommand> [OPTIONS] <archive>                        # info, list, verify, compact
mpqcli <subcommand> [OPTIONS] <archive> <files>                # create, add, remove
mpqcli extract      [OPTIONS] <archive> [files]                # extract (omitted = all)
mpqcli read         [OPTIONS] <archive> <file>                 # read (single file)
mpqcli rename       [OPTIONS] <archive> <old-file> <new-file>

[thomaslaurenson]

Hey @sjoblomj - tagging you in this issue, as I think you will be interested in this proposal, and you always have good input on the design for this project 😄

[thomaslaurenson]

Sorry for all the comment replies. I got focused on the positional arguments problem and forgot to mention... I agree that automated slash conversion and handling should be done by mpqcli. Happy to add that to the backlog. I created issue #181 to track

[sjoblomj]

Thank you for bringing this up, @MikaMika , and thank you for such a detailed and well thought through proposal, @thomaslaurenson !

Strong agree to all your suggestions. I also think it would be very good to settle on "archive" for the archive parameter - I've tripped many times on what the "target" is when writing tests. 🙃

The only questions I have is for create: IIRC you can currently do create mydir, which puts everything under mydir into mydir.mpq. Is this behaviour that should be kept, or will we now start forcing the user to write create mydir.mpq mydir? I think that would be fine, but it's a little ambiguous from your proposal.

It would be good to compare how Unix utils like zip or tar handle similar parameters. Would mpqcli now have identical behaviour or do they do parameters differently?

[thomaslaurenson]

Argument naming convention

• I tried to find a better name than "files" to differentiate "on disk" to "in archive"
• Finally settled on the fact that the subcommand (verb) indicates what type of files they are
• add will put local files in an archive
• remove will take out in archive from from an archive
• This seemed to make sense to me, and the cleanest solution I could come up with

Tool	Archive arg	File on disk arg	File in archive arg
GNUtar	ARCHIVE	FILE...	MEMBER...
zip	zipfile	file...	file...
unzip	file[.zip]	-	file(s)...
zipinfo	file[.zip]	-	file(s)...
7z	<archive_name>	<file_names>...	<file_names>...
jar	--filefoo.jar	files	files
cabextract	cabinet	-	files...
ar	archive	member...	member...

Argument order

• I mainly went with archive first, as it seems to be generally the consensus
• Additionally, it makes it way easier to have multiple "file" arguments

tar   [options] -f ARCHIVE FILE...        # archive named via -f flag, not positional; files positional after
zip   [options] zipfile.zip file...       # archive first, then files
unzip [options] file.zip [file...]        # archive first, then files (to extract)
7z    command [switches] archive_name [file_names...]   # archive first, then files
ar    [options] [member-name] archive-file file...      # archive-file, THEN files (member-name is an option arg, different feature)

Create command

• Hmmmm, this is a good question about the create command structure...
• The current design say yes, you need to include the archive as the first positional argument, it will no longer be derived from the directory argument
• I think this seems OK, as it makes all the subcommands the same structure, but open to feedback