Adding 'overwrite', 'name-in-archive' and 'dir-in-archive' arguments to the add command #115

sjoblomj · 2025-10-18T11:23:48Z

The add command now accepts these arguments:

overwrite - if set to true, an existing file inside the MPQ will be overwritten.
name-in-archive - the complete filename of the added file inside the MPQ.
dir-in-archive - the directory name to place the file under inside the MPQ. Previously called path. I find it clearer to rename the argument to dir-in-archive since that is what it is, and the Readme clearly states that things might change until v1.0.0.

In addition, I refactored the tests for the add command and corrected a previous test that didn't do what it was supposed to.

sjoblomj · 2025-10-18T11:43:22Z

I would actually argue that the dir-in-archive argument is superfluous when a name-in-archive can be given, so I'd recommend removing dir-in-archive altogether to keep the UI simple. But I'll let others decide that.

thomaslaurenson

Sorry it took so long for me to look at this PR. I add a couple comments. All looks good, and only problem I have with the addition is the argument names. I added a couple options, but think that -f,--file and -p,--path might be the best (easiest to read/understand) option. Also, thank you for adding more tests for this - amazing 🥇

EDIT: Maybe -f,--filename might be better

thomaslaurenson · 2025-10-31T19:00:16Z

src/main.cpp

-    add->add_option("-p,--path", basePath, "Path within MPQ archive");
+    add->add_option("--dir-in-archive", baseDirInArchive, "Directory to put file inside within MPQ archive");
+    add->add_option("--name-in-archive", baseNameInArchive, "Filename inside MPQ archive");
+    add->add_flag("--overwrite", addOverwrite, "Overwrite file if it already is in MPQ archive");


Can you please add in -o as a short option. Would be nice if every argument had a short option

I'm happy to add it. The reason I did not is because -o is used as short option for --output in the subcommands extract and create. While this is for the add subcommand - so it should be unambiguous - I still figured it could be confusing to have the same option mean different things in different contexts.

If you think it might be problematic to have -o mean multiple things, we could maybe use -w as short hand for --overwrite? If not, I'm happy to add -o here too -- I have no strong feelings about it :)

Let me know what you prefer.

I added -w as a short option for overwrite (see my above comment as to why). Let me know if you prefer -o instead.

thomaslaurenson · 2025-10-31T19:03:13Z

README.md

-mpqcli add fta.txt wow-patch.mpq --path <path/in/archive>
+$ echo "For The Alliance" > fta.txt
+$ mpqcli add fta.txt wow-patch.mpq --name-in-archive "texts\\alliance.txt"
+[+] Adding file: texts\alliance.txt


Hmmmm.... I am both sides of the fence about adding $ to indicate a prompt (when a command is being run). It makes an example like this much easier to read (two lines of commands)... But makes it harder to copy paste. Maybe the better solution would be:

echo "For The Alliance" > fta.txt; mpqcli add fta.txt wow-patch.mpq --name-in-archive "texts\\alliance.txt" [+] Adding file: texts\alliance.txt

Anyway - no big deal about this one. Feel free to leave or change

Since the examples mix commands and their output, I figured it would be good to make it clear what is what. I suppose that by chaining commands and thus making it a one liner, it does become pretty clear, but readability suffers.

I'd keep the $ , but this is very minor, and I don't mind either way.

thomaslaurenson · 2025-10-31T19:11:49Z

src/main.cpp

        ->check(CLI::ExistingFile);
-    add->add_option("-p,--path", basePath, "Path within MPQ archive");
+    add->add_option("--dir-in-archive", baseDirInArchive, "Directory to put file inside within MPQ archive");
+    add->add_option("--name-in-archive", baseNameInArchive, "Filename inside MPQ archive");


Not sure about the argument naming here. I have no problem with the logic (implemented below). But the long command arguments are a little unwieldy.

Was thinking archive-name and archive-path, but these are confusing - as people may think that is the actual MPQ archive name/path.

Maybe path and name would be simple?

mpqcli add -p path/in/archive -f filename-in-archive.exe targetfile.exe target.mpq

This might work, as the actual local file to add is a positional argument (so -f and --file is not used). I am not sure, this might also be confusing to users, but is the best one I could come up with.

Thoughts?

I agree with you that the long command arguments are a little unwieldy. But like you also say, it's difficult to find good shorter names that are not potentially unclear. I find archive-name and archive-path downright confusing, and path and name (or filename) rather non-descriptive.

My take would be this:

Remove path / dir-in-archive, since the same can be accomplished by the name-in-archive parameter. Removing it keeps the interface simpler.

Keep the name-in-archive name even though its longer than ideal.

Add a short option -n (or -f) for name-in-archive. This way it's convenient to use it, even though the long argument name is unwieldy.

These are my recommendations. However, I'm not very vested in the naming here, and I'm happy to let you have the final say in what to name the arguments, their short options, and whether to keep or remove path / dir-in-archive.

sjoblomj · 2025-11-01T10:39:02Z

Sorry it took so long for me to look at this PR. I add a couple comments. All looks good, and only problem I have with the addition is the argument names. I added a couple options, but think that -f,--file and -p,--path might be the best (easiest to read/understand) option. Also, thank you for adding more tests for this - amazing 🥇

EDIT: Maybe -f,--filename might be better

Thank you, happy you like it. I gave my thoughts on the argument naming in the comments above.

I think merging all my open PRs would lead to minor merge conflicts as it stands right now. To keep things simple, let's first merge #117 and the locale PRs, once you have checked them and agree with them. Afterwards, I can rebase and update this PR and #116.
Edit: Locale PRs are merged and this PR is updated to fix merge conflicts.

…to the add command

sjoblomj · 2025-11-27T18:00:18Z

@thomaslaurenson : I fixed the merge conflicts (both here and in #116). Both PRs are pretty much identical to before. Let me know what you think about the naming of parameters, and whether or not to keep --path / --dir-in-archive.

Foereaper · 2025-12-07T21:58:09Z

I'd prefer to keep the path command so I don't have to construct the path and append the filename to said path for any file I'd add to a specific directory :)

sjoblomj · 2025-12-07T23:20:04Z

I'd prefer to keep the path command so I don't have to construct the path and append the filename to said path for any file I'd add to a specific directory :)

I understand its usefulness, but something like either of these would accomplish the same, while hardly being heavier on the user:

filename=some-file.txt && mpqcli add "$filename" wow-patch.mpq --name-in-archive "some-dir\\$filename"
for filename in a/bunch/of/files/to/add/* ; do mpqcli add "$filename" wow-patch.mpq --name-in-archive "some-dir\\$filename" ; done

This is arguably somewhat more cumbersome for the user, but the readme says it "is a command-line tool, designed for automation", and it is "designed to work seamlessly with other command-line tools, supporting piping, redirection, and integration into shell scripts and workflows".

In my mind, that means the --path is a bit superfluous, but again, I'm happy either way.

Foereaper · 2025-12-08T15:22:47Z

I'd prefer to keep the path command so I don't have to construct the path and append the filename to said path for any file I'd add to a specific directory :)

I understand its usefulness, but something like either of these would accomplish the same, while hardly being heavier on the user:

filename=some-file.txt && mpqcli add "$filename" wow-patch.mpq --name-in-archive "some-dir\\$filename"

for filename in a/bunch/of/files/to/add/* ; do mpqcli add "$filename" wow-patch.mpq --name-in-archive "some-dir\\$filename" ; done

This is arguably somewhat more cumbersome for the user, but the readme says it "is a command-line tool, designed for automation", and it is "designed to work seamlessly with other command-line tools, supporting piping, redirection, and integration into shell scripts and workflows".

In my mind, that means the --path is a bit superfluous, but again, I'm happy either way.

From an end user perspective I'd personally say that keeping the path and name in archive separate is more intuitive than having a combined option where the end user has to construct the final path + filename, even if that is how MPQs handle files internally. I think it is better to handle the merging of the filename and the path internally in the tool, rather than having to do so externally. It's a nice abstraction with clear delineation, and allows for some more granularity, but either way I'm happy with the option to specify the name of the file!

sjoblomj changed the title ~~Adding filename and dir arguments to the add command~~ Adding 'overwrite', 'filename' and 'dir' arguments to the add command Oct 19, 2025

sjoblomj force-pushed the filename-and-dir-arguments-to-add branch from 8e23498 to a64d52d Compare October 19, 2025 09:32

sjoblomj changed the title ~~Adding 'overwrite', 'filename' and 'dir' arguments to the add command~~ Adding 'overwrite', 'name-in-archive' and 'dir-in-archive' arguments to the add command Oct 19, 2025

thomaslaurenson requested changes Oct 31, 2025

View reviewed changes

thomaslaurenson mentioned this pull request Oct 31, 2025

Create command can now take single file; adding --name-in-archive parameter #116

Open

sjoblomj mentioned this pull request Nov 24, 2025

Locale support - extract #121

Merged

sjoblomj force-pushed the filename-and-dir-arguments-to-add branch from 41dd245 to 3f9251e Compare November 27, 2025 17:47

Adding 'overwrite', 'name-in-archive' and 'dir-in-archive' arguments …

91dbc30

…to the add command

sjoblomj force-pushed the filename-and-dir-arguments-to-add branch from 3f9251e to 91dbc30 Compare November 27, 2025 17:49

Adding 'overwrite', 'name-in-archive' and 'dir-in-archive' arguments to the add command #115

Are you sure you want to change the base?

Adding 'overwrite', 'name-in-archive' and 'dir-in-archive' arguments to the add command #115

Uh oh!

Conversation

sjoblomj commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjoblomj commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomaslaurenson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomaslaurenson Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

sjoblomj Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjoblomj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

thomaslaurenson Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

thomaslaurenson Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

sjoblomj Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

thomaslaurenson Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

sjoblomj Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjoblomj commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjoblomj commented Nov 27, 2025

Uh oh!

Foereaper commented Dec 7, 2025

Uh oh!

sjoblomj commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Foereaper commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sjoblomj commented Oct 18, 2025 •

edited

Loading

sjoblomj commented Oct 18, 2025 •

edited

Loading

thomaslaurenson left a comment •

edited

Loading

sjoblomj Nov 1, 2025 •

edited

Loading

sjoblomj Nov 1, 2025 •

edited

Loading

sjoblomj commented Nov 1, 2025 •

edited

Loading

sjoblomj commented Dec 7, 2025 •

edited

Loading