Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix 3954 quotes in merge commit message #8042

Merged
merged 2 commits into from Apr 28, 2020
Merged

Fix 3954 quotes in merge commit message #8042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54b53d6
fix: Can't have quotes in merge-commit message
RussKie Apr 26, 2020
d3fd17b
fix: Tighten git-merge log condition
RussKie Apr 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
95 changes: 83 additions & 12 deletions GitCommands/CommitMessageManager.cs
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ public interface ICommitMessageManager
bool AmendState { get; set; }

/// <summary>
/// The pathname of the file where a prepared (non-merge) commit message is stored.
/// The path of .git/COMMITMESSAGE, where a prepared (non-merge) commit message is stored.
/// </summary>
string CommitMessagePath { get; }

Expand All @@ -24,6 +24,11 @@ public interface ICommitMessageManager
/// </summary>
bool IsMergeCommit { get; }

/// <summary>
/// The path of .git/MERGE_MSG, where a merge-commit message is stored.
/// </summary>
string MergeMessagePath { get; }

/// <summary>
/// Reads/stores the prepared commit message from/in .git/MERGE_MSG if it exists or else in .git/COMMITMESSAGE.
/// </summary>
Expand All @@ -33,6 +38,16 @@ public interface ICommitMessageManager
/// Deletes .git/COMMITMESSAGE and the file with the AmendState.
/// </summary>
void ResetCommitMessage();

/// <summary>
/// Writes the provided commit message to .git/COMMITMESSAGE.
/// The message is formatted depending whether a commit template is used or whether the 2nd line must be empty.
/// </summary>
/// <param name="commitMessage">The commit message to write out.</param>
/// <param name="messageType">The type of message to write out.</param>
/// <param name="usingCommitTemplate">The indicator whether a commit tempate is used.</param>
/// <param name="ensureCommitMessageSecondLineEmpty">The indicator whether empty second line is enforced.</param>
void WriteCommitMessageToFile(string commitMessage, CommitMessageType messageType, bool usingCommitTemplate, bool ensureCommitMessageSecondLineEmpty);
}

public sealed class CommitMessageManager : ICommitMessageManager
Expand All @@ -42,11 +57,16 @@ public sealed class CommitMessageManager : ICommitMessageManager
private string CannotAccessFile => "Exception: \"{0}\" when accessing {1}";

private readonly string _amendSaveStatePath;
private readonly string _commitMessagePath;
private readonly string _mergeMessagePath;

private Encoding _commitEncoding;
private IFileSystem _fileSystem;
private readonly IFileSystem _fileSystem;

// Commit messages are UTF-8 by default unless otherwise in the config file.
// The git manual states:
// git commit and git commit-tree issues a warning if the commit log message
// given to it does not look like a valid UTF-8 string, unless you
// explicitly say your project uses a legacy encoding. The way to say
// this is to have i18n.commitencoding in .git/config file, like this:...
private readonly Encoding _commitEncoding;

[CanBeNull]
private string _overriddenCommitMessage;
Expand All @@ -61,11 +81,12 @@ internal CommitMessageManager(string workingDirGitDir, Encoding commitEncoding,
_fileSystem = fileSystem;
_commitEncoding = commitEncoding;
_amendSaveStatePath = GetFilePath(workingDirGitDir, "GitExtensions.amend");
_commitMessagePath = GetFilePath(workingDirGitDir, "COMMITMESSAGE");
_mergeMessagePath = GetFilePath(workingDirGitDir, "MERGE_MSG");
CommitMessagePath = GetFilePath(workingDirGitDir, "COMMITMESSAGE");
MergeMessagePath = GetFilePath(workingDirGitDir, "MERGE_MSG");
_overriddenCommitMessage = overriddenCommitMessage;
}

/// <inheritdoc/>
public bool AmendState
{
get
Expand All @@ -92,10 +113,16 @@ public bool AmendState
}
}

public string CommitMessagePath => _commitMessagePath;
/// <inheritdoc/>
public string CommitMessagePath { get; }

/// <inheritdoc/>
public bool IsMergeCommit => _fileSystem.File.Exists(MergeMessagePath);

public bool IsMergeCommit => _fileSystem.File.Exists(_mergeMessagePath);
/// <inheritdoc/>
public string MergeMessagePath { get; }

/// <inheritdoc/>
public string MergeOrCommitMessage
{
get
Expand Down Expand Up @@ -149,23 +176,67 @@ public string MergeOrCommitMessage
}
}

/// <inheritdoc/>
public void ResetCommitMessage()
{
_overriddenCommitMessage = null;
_fileSystem.File.Delete(_commitMessagePath);
_fileSystem.File.Delete(CommitMessagePath);
_fileSystem.File.Delete(_amendSaveStatePath);
}

/// <inheritdoc/>
public void WriteCommitMessageToFile(string commitMessage, CommitMessageType messageType, bool usingCommitTemplate, bool ensureCommitMessageSecondLineEmpty)
{
var formattedCommitMessage = FormatCommitMessage(commitMessage, usingCommitTemplate, ensureCommitMessageSecondLineEmpty);

string path = messageType == CommitMessageType.Normal ? CommitMessagePath : MergeMessagePath;
File.WriteAllText(path, formattedCommitMessage, _commitEncoding);
}

internal static string FormatCommitMessage(string commitMessage, bool usingCommitTemplate, bool ensureCommitMessageSecondLineEmpty)
{
if (string.IsNullOrEmpty(commitMessage))
{
return string.Empty;
}

var formattedCommitMessage = new StringBuilder();

var lineNumber = 1;
foreach (var line in commitMessage.Split('\n'))
{
string nonNullLine = line ?? string.Empty;

// When a committemplate is used, skip comments and do not count them as line.
// otherwise: "#" is probably not used for comment but for issue number
if (usingCommitTemplate && nonNullLine.StartsWith("#"))
{
continue;
}

if (ensureCommitMessageSecondLineEmpty && lineNumber == 2 && !string.IsNullOrEmpty(nonNullLine))
{
formattedCommitMessage.AppendLine();
}

formattedCommitMessage.AppendLine(nonNullLine);

lineNumber++;
}

return formattedCommitMessage.ToString();
}

private string GetFilePath(string workingDirGitDir, string fileName) => _fileSystem.Path.Combine(workingDirGitDir, fileName);

private (string filePath, bool fileExists) GetMergeOrCommitMessagePath()
{
if (IsMergeCommit)
{
return (_mergeMessagePath, fileExists: true);
return (MergeMessagePath, fileExists: true);
}

return (_commitMessagePath, _fileSystem.File.Exists(_commitMessagePath));
return (CommitMessagePath, _fileSystem.File.Exists(CommitMessagePath));
}
}
}
8 changes: 8 additions & 0 deletions GitCommands/CommitMessageType.cs
View file
Open in desktop
@@ -0,0 +1,8 @@
namespace GitCommands
{
public enum CommitMessageType
{
Normal = 0,
Merge
}
}
8 changes: 3 additions & 5 deletions GitCommands/Git/GitCommandHelpers.cs
View file
Open in desktop
Expand Up @@ -1048,19 +1048,17 @@ private static GitItemStatus GitItemStatusFromStatusCharacter(StagedStatus stage
};
}

public static ArgumentString MergeBranchCmd(string branch, bool allowFastForward, bool squash, bool noCommit, string strategy, bool allowUnrelatedHistories, string message, int? log)
public static ArgumentString MergeBranchCmd(string branch, bool allowFastForward, bool squash, bool noCommit, string strategy, bool allowUnrelatedHistories, string mergeCommitFilePath, int? log)
{
// TODO Quote should (optionally?) escape any " characters, at least for usages like the below

return new GitArgumentBuilder("merge")
{
{ !allowFastForward, "--no-ff" },
{ !string.IsNullOrEmpty(strategy), $"--strategy={strategy}" },
{ squash, "--squash" },
{ noCommit, "--no-commit" },
{ allowUnrelatedHistories, "--allow-unrelated-histories" },
{ !string.IsNullOrEmpty(message), $"-m {message.Quote()}" },
{ log != null, $"--log={log}" },
{ !string.IsNullOrWhiteSpace(mergeCommitFilePath), $"-F \"{mergeCommitFilePath}\"" }, // let git fail, if the file doesn't exist
{ log != null && log.Value > 0, $"--log={log}" },
branch
};
}
Expand Down
69 changes: 6 additions & 63 deletions GitUI/CommandsDialogs/FormCommit.cs
View file
Open in desktop
Expand Up @@ -16,15 +16,13 @@
using GitCommands.Config;
using GitCommands.Git;
using GitCommands.Patches;
using GitCommands.Settings;
using GitCommands.Utils;
using GitExtUtils;
using GitExtUtils.GitUI;
using GitExtUtils.GitUI.Theming;
using GitUI.AutoCompletion;
using GitUI.CommandsDialogs.CommitDialog;
using GitUI.Editor;
using GitUI.HelperDialogs;
using GitUI.Hotkey;
using GitUI.Properties;
using GitUI.Script;
Expand Down Expand Up @@ -1413,7 +1411,12 @@ void DoCommit()
{
if (_useFormCommitMessage)
{
SetCommitMessageFromTextBox(Message.Text);
// Save last commit message in settings. This way it can be used in multiple repositories.
AppSettings.LastCommitMessage = Message.Text;

_commitMessageManager.WriteCommitMessageToFile(Message.Text, CommitMessageType.Normal,
usingCommitTemplate: !string.IsNullOrEmpty(_commitTemplate),
ensureCommitMessageSecondLineEmpty: AppSettings.EnsureCommitMessageSecondLineEmpty);
}

ScriptManager.RunEventScripts(this, ScriptEvent.BeforeCommit);
Expand Down Expand Up @@ -2244,62 +2247,6 @@ private void EditGitInfoExcludeToolStripMenuItemClick(object sender, EventArgs e
Initialize();
}

private static string FormatCommitMessageFromTextBox(
string commitMessageText, bool usingCommitTemplate, bool ensureCommitMessageSecondLineEmpty)
{
if (commitMessageText == null)
{
return string.Empty;
}

var formattedCommitMessage = new StringBuilder();

var lineNumber = 1;
foreach (var line in commitMessageText.Split('\n'))
{
string nonNullLine = line ?? string.Empty;

// When a committemplate is used, skip comments and do not count them as line.
// otherwise: "#" is probably not used for comment but for issue number
if (usingCommitTemplate && nonNullLine.StartsWith("#"))
{
continue;
}

if (ensureCommitMessageSecondLineEmpty && lineNumber == 2 && !string.IsNullOrEmpty(nonNullLine))
{
formattedCommitMessage.AppendLine();
}

formattedCommitMessage.AppendLine(nonNullLine);

lineNumber++;
}

return formattedCommitMessage.ToString();
}

private void SetCommitMessageFromTextBox(string commitMessageText)
{
// Save last commit message in settings. This way it can be used in multiple repositories.
AppSettings.LastCommitMessage = commitMessageText;

var path = _commitMessageManager.CommitMessagePath;

var formattedCommitMessage = FormatCommitMessageFromTextBox(
commitMessageText, usingCommitTemplate: !string.IsNullOrEmpty(_commitTemplate), AppSettings.EnsureCommitMessageSecondLineEmpty);

// Commit messages are UTF-8 by default unless otherwise in the config file.
// The git manual states:
// git commit and git commit-tree issues a warning if the commit log message
// given to it does not look like a valid UTF-8 string, unless you
// explicitly say your project uses a legacy encoding. The way to say
// this is to have i18n.commitencoding in .git/config file, like this:...
Encoding encoding = Module.CommitEncoding;

File.WriteAllText(path, formattedCommitMessage, encoding);
}

private void DeleteAllUntrackedFilesToolStripMenuItemClick(object sender, EventArgs e)
{
if (MessageBox.Show(this,
Expand Down Expand Up @@ -3355,10 +3302,6 @@ internal TestAccessor(FormCommit formCommit)
internal CommandStatus ExecuteCommand(Command command) => _formCommit.ExecuteCommand((int)command);

internal Rectangle Bounds => _formCommit.Bounds;

internal static string FormatCommitMessageFromTextBox(
string commitMessageText, bool usingCommitTemplate, bool ensureCommitMessageSecondLineEmpty)
=> FormCommit.FormatCommitMessageFromTextBox(commitMessageText, usingCommitTemplate, ensureCommitMessageSecondLineEmpty);
}
}

Expand Down
16 changes: 15 additions & 1 deletion GitUI/CommandsDialogs/FormMergeBranch.cs
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ public partial class FormMergeBranch : GitModuleForm
private readonly TranslationString _strategyTooltipText = new TranslationString("resolve " + Environment.NewLine + "This can only resolve two heads (i.e. the current branch and another branch you pulled from) using a 3-way merge algorithm." + Environment.NewLine + "It tries to carefully detect criss-cross merge ambiguities and is considered generally safe and fast. " + Environment.NewLine + Environment.NewLine + "recursive " + Environment.NewLine + "This can only resolve two heads using a 3-way merge algorithm. When there is more than one common ancestor that can be " + Environment.NewLine + "used for 3-way merge, it creates a merged tree of the common ancestors and uses that as the reference tree for the 3-way" + Environment.NewLine + "merge. Additionally this can detect and handle merges involving renames. This is the default merge strategy when pulling or " + Environment.NewLine + "merging one branch. " + Environment.NewLine + Environment.NewLine + "octopus " + Environment.NewLine + "This resolves cases with more than two heads, but refuses to do a complex merge that needs manual resolution. It is " + Environment.NewLine + "primarily meant to be used for bundling topic branch heads together. This is the default merge strategy when pulling or " + Environment.NewLine + "merging more than one branch. " + Environment.NewLine + Environment.NewLine + "ours " + Environment.NewLine + "This resolves any number of heads, but the resulting tree of the merge is always that of the current branch head, effectively " + Environment.NewLine + "ignoring all changes from all other branches. It is meant to be used to supersede old development history of side branches." + Environment.NewLine + Environment.NewLine + "subtree " + Environment.NewLine + "This is a modified recursive strategy. When merging trees A and B, if B corresponds to a subtree of A, B is first adjusted to " + Environment.NewLine + "match the tree structure of A, instead of reading the trees at the same level. This adjustment is also done to the common " + Environment.NewLine + "ancestor tree.");
private readonly TranslationString _formMergeBranchHoverShowImageLabelText = new TranslationString("Hover to see scenario when fast forward is possible.");
private readonly string _defaultBranch;
private ICommitMessageManager _commitMessageManager;

[Obsolete("For VS designer and translation test only. Do not remove.")]
private FormMergeBranch()
Expand All @@ -31,6 +32,8 @@ public FormMergeBranch(GitUICommands commands, string defaultBranch)
helpImageDisplayUserControl1.Image2 = Properties.Images.HelpCommandMergeFastForward.AdaptLightness();
InitializeComplete();

_commitMessageManager = new CommitMessageManager(Module.WorkingDirGitDir, Module.CommitEncoding);

currentBranchLabel.Font = new Font(currentBranchLabel.Font, FontStyle.Bold);
noCommit.Checked = AppSettings.DontCommitMerge;

Expand Down Expand Up @@ -83,13 +86,24 @@ private void OkClick(object sender, EventArgs e)
AppSettings.DontCommitMerge = noCommit.Checked;
ScriptManager.RunEventScripts(this, ScriptEvent.BeforeMerge);

string mergeMessagePath = null;
if (addMergeMessage.Checked)
{
// [!] Do not reset the last commit message stored in AppSettings.LastCommitMessage

_commitMessageManager.WriteCommitMessageToFile(mergeMessage.Text, CommitMessageType.Merge,
usingCommitTemplate: false,
ensureCommitMessageSecondLineEmpty: false);
mergeMessagePath = _commitMessageManager.MergeMessagePath;
}

var successfullyMerged = FormProcess.ShowDialog(this, GitCommandHelpers.MergeBranchCmd(Branches.GetSelectedText(),
fastForward.Checked,
squash.Checked,
noCommit.Checked,
_NO_TRANSLATE_mergeStrategy.Text,
allowUnrelatedHistories.Checked,
addMergeMessage.Checked ? mergeMessage.Text : null,
mergeMessagePath,
addLogMessages.Checked ? (int)nbMessages.Value : (int?)null));

var wasConflict = MergeConflictHandler.HandleMergeConflicts(UICommands, this, !noCommit.Checked);
Expand Down