@m3nu This is ready for review.

Thanks for the PR, as always. So this aims to log a command for copy & pasting and reversing some of the escaping done when logging this? I wonder if there is a simpler way to achieve something similar? E.g. here some different ways to log stuff.

# This WILL escape the newline because %r uses repr(msg).
logging.debug("Repr style: %r", msg)

# This will NOT escape the newline; you'll actually see the newline rendered.
logging.debug("String style: %s", msg)

The issue could also come from our join() function.

For this PR I worry that:

• It's pretty complex and runs for each Borg command, even though it's rarely needed.
• If there is any issue in this function, the Borg command will fail.
• No tests to ensure the examples work.
• There may be more cases that need be added now or in the future. I think building a function that reverses some other escaping function is pretty complex.
• I wouldn't be able to debug it in the future and would likely just remove it, if there is any issue.

@m3nu: Thanks for the feedback.

Printing the list of borg command arguments is a simple one-liner:
logger.info('borg command arguments: %s', self.cmd)
This prints the exact list of borg arguments as passed to Popen() - this is everything that is required in the logs for debugging.

So a simple (and correct) solution would be to just put this single line instead of (re-)constructing a valid shell command.
Disadvantage: This list of "borg command arguments" cannot be copied/ pasted to a shell (not sure if this is even a goal)

The list of borg command arguments does not contain any escape characters or quotation marks. These are not required if no shell is involved.
So whatever we do when building a valid shell command for the logs - we have to manually take care about how we treat whitespace and special characters:

• Thomas Waldman pointed out that whitespace should not be escaped by "\" since this is not supported by all shells.
• Moreover, there may be wildcards and regular expression which also should be put into quotation marks - for shell commands (this is written in the borg docs).

Computational overhead: I wondered about this, too. :-) To get a rough estimate, I run the code to format the pretty long and complicated list of borg arguments from my commit message, on my mid-range laptop inside a Jupyter notebook.
It took 0.13 milliseconds - not sure if this is acceptable, or not.

Complexity I agree that the code in the pull request is not self-explanatory, since I tried to put the first quotation mark after the first "=" if there are any.
Thinking about it - one way to reduce the complexity is to just always put the whole argument inside quotation marks, whenever it contains one of the special strings - which would result in a pretty self-explanatory four-liner, since I would get rid of the "if elseif else craziness":

special_strings = [' ','*', '?', 're:']
for i, arg in enumerate(cmd):
     if any(specialstr in arg for specialstr in special_strings):
        cmd[i]=arg.replace(arg,"\'"+arg+"\'")    #add quotation marks 

The only difference for the shell command approximation would be that with the simplified code I would get
'--pattern=-home/user/test folder'
instead of:
--pattern='-home/user/test folder'
Which both run just fine in the shell, despite the whitespace. :-)

Currently, in 0.10.3, we write a borg command (approximation) to the logs that escape whitespace using "\" and without any quotation marks (not even for regular expressions "re:") - I would like to fix this.

I believe there are several valid options, here:

1. Just print the plain list of borg arguments
2. Simplify the pull request and only put the shell command (approximation)
3. Put both
4. ?

Any preferences?

Thinking about this again, I believe option 2 would be a good one:

• The only change would be that in the logs, the borg arguments are put into quotation marks if they contain
  special strings (i.e. whitespace, wildcards or regular expressions)
• as a result the arguments are preserved (not changed), which is good for debugging
• the resulting borg command, in the logs, could be used as a shell command

@m3nu I simplified the code and updated my pull request:

Changes:

• Simplified logic: just put the whole argument within quotation marks if it contains whitespace, wildcards or regular expressions
  • good for debugging, since argument splitting and arguments themselves are not touched
  • no complicated if/else if/else construct :-)
  • code is pretty much self-explanatory (IMO) - it would be easy to e.g. add a new "quotation_string".
• the resulting borg command from the logs can be copied/ pasted/executed in a shell as is
• computational time is down by half, to 0,0731 milliseconds for complicated cases

This solution is more robust than what is in 0.10.3:

• the use of escape characters "\" for whitespace in 0.10.3 is shell dependent.
• currently, the borg command in the logs is printed without any quotation marks.

Example log output with the modified pull request - for a complicated case:
/usr/bin/borg create --remote-path=/usr/local/bin/borg --list --progress --info --log-json --json --filter=AM -C lz4 '--pattern=+home/user/test folder/testfile' '--pattern=-home/user/test folder' '--pattern=-home/user/test folder' '--pattern=-re:home/user/test\sfolder2' '--pattern=-re:crazy?=cra' -e 're:crazy?=cra' -e '/home/user/*.tmp' -e /home/user/test/cache --patterns-from /home/user/patterns.lst ssh://[email protected]:22/~/backup/vorta-repo::user-845-2025-01-15-083513 /home/user

This command can be copied pasted from the logs to the shell - and runs just fine (only archive name has to be updated, obviously)

As before: The execution path is not concerned here - this is purely about a pretty and useful (for debugging and copy) borg command in the logs.

The new version is much clearer! No objections to add this.

The corresponding issue has now automatically been "marked as stale":
#2189 (comment)

This pull request has been marked as approved on Jan 16 - does that mean it will be merged automatically, later?
Or is there something missing, from my side?

Can you review this @VandalByte? I don't want any new errors just because we're formatting logs differently. After that I'm happy to merge it. Thanks!

When it comes to errors, I ran tests with multiple flags and various borg commands, and I didn’t encounter any issues.

Some things I want to point out, as goebbe mentioned:

The only difference for the shell command approximation would be that with the simplified code I would get
'--pattern=-home/user/test folder'
instead of:
--pattern='-home/user/test folder'
Which both run just fine in the shell, despite the whitespace. :-)

Flags that have a similar pattern like --pattern=, --remote-path= are getting fully quoted, if the code encounters any char from quote_strings.

Actual -> '--pattern=- /home/vandal/bb/file3.txt'
Expected -> --pattern='- /home/vandal/bb/file3.txt'

I saw that code to handle that was removed from previous commit. Even though argparse might throw an error on something like this, borg seems to handle it fine internally.

Example, I tested it out by running:

/usr/bin/borg create --list --progress --info --log-json --json --filter=AM -C lz4 '--pattern=- /home/vandal/bb/file3.txt'   /home/vandal/Music/secret::litc1   /home/vandal/bb

And this is the expected command:

/usr/bin/borg create --list --progress --info --log-json --json --filter=AM -C lz4 --pattern='- /home/vandal/bb/file3.txt'   /home/vandal/Music/secret::litc2   /home/vandal/bb

Both worked as expected and ignored file3.txt, which was confirmed from nfiles=2 (the directory had 3 files in total). And when the flag was left out all 3 files were added as expected.

As of right now borg seems to handle the fully quoted flags properly on their end based on my testing. Unless something changes in future versions, this borg command being shown in logs appears to work fine as expected.

@VandalByte Thanks for reviewing the code!

I saw that code to handle that was removed from previous commit. Even though argparse might throw an error on something like this, borg seems to handle it fine internally.
...
As of right now borg seems to handle the fully quoted flags properly on their end based on my testing. Unless something changes in future versions, this borg command being shown in logs appears to work fine as expected.

I simplified the code of this pull request by quoting the whole argument, which works just fine with borg.

The idea to use this solution (simplification) came from a related bug report on borg: borgbackup/borg#8578 (comment)
There, cdchristensen pointed out that this would be one supported way for quoting special strings in borg.
Thomas Waldman (the main dev of borgbackup) gave thumps up. Therefore, I guess this type of quoting should be fine.

I also ran a couple of tests to check if I can copy/ paste / run the borg command from the logs directly in a terminal - and it seems to work just fine.

@goebbe I tested it out as well and everything looks good! 👍

Sorry to be a stickler, but the linting test failed, try running pre-commit after staging the changes. It's probably due to the double space before the comment.

Thanks for the addition @goebbe and for reviewing @VandalByte!