Pass the git commit message via a file

[evelikov]

Currently we pass it via the command line, which causes a bunch of grief. Namely: various characters need to be escaped since they will conflict with the system's restrictions.

Avoid all that and use the --file instead of --message

As a follow-up the no longer required sanitization will be removed.

Issue: https://github.com/hbons/SparkleShare/issues/1788 Signed-off-by: Emil Velikov emil.velikov@collabora.com

[evelikov]

Note that I haven't removed any sanitization since I don't know which can go/stay. Some educated guesswork suggests Replace and EnsureSpecialChars instances like below.

https://github.com/hbons/SparkleShare/blob/4e0dff6956c7c78858c3929650554cb302c29634/Sparkles/Git/GitRepository.cs#L208

https://github.com/hbons/SparkleShare/blob/4e0dff6956c7c78858c3929650554cb302c29634/Sparkles/Git/GitRepository.cs#L1047

[hbons]

This is a good idea!

[evelikov]

Patch updated, changes:

• fold the GetTempFileName method within Commit
• create file within .git/sparkleshare/
• bump number of attempts
• improve coding style

[hbons]

@evelikov I've taken your ideas and put them in #1853. The file handling is a bit more straightforward. See https://github.com/hbons/SparkleShare/pull/1853/commits/f6b413ca8e70e182eafea9522327774cdece4d5b. The PR also removes some of the special character parsing and other hacks, but there may be more.

[evelikov]

@hbons as mentioned before adding non-namespaced files in .git is a bad idea. Furthermore there's no guarantee that tomorrow git won't start creating a file that clashes with the one you add. Secondary, the "Changes by SparkleShare" is completely useless and one might as well not have any.

The "complexity" comes from the fact that C# lacks simple yet robust API like mkstemp mentioned earlier.

[BarryThePenguin]

Squirrel has some useful utility methods for managing temporary files and directories

https://github.com/Squirrel/Squirrel.Windows/blob/d6282091c81c12c6fd952da17e11a1781b7b13c0/src/Squirrel/Utility.cs#L244-L293

[hbons]

@evelikov I get not using a namespace was not the best decision, but it will take some more effort to move everything (specially existing installations) to the new namespace. I don't think it's worth the effort right now.

In any case. Thanks for bringing up this issue and proposed solutions. Much appreciated. :)

[evelikov]

@hbons a couple of suggestions for the future:

• don't say "you can review it" if you're going to ignore any of the feedback
• when building onto someone else's work, credit them

[hbons]

@evelikov Sorry totally forgot! I've made a new commit because it's so you show up in the log/blame: https://github.com/hbons/SparkleShare/commit/d4536dfd62b4d760878a1d2bcdd69b7cc31bc59e