Skip to content

Commit

Permalink
Fix resetting selected lines for renamed files in index (#10252)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 69a8570)
  • Loading branch information
@mdonatas @RussKie
mdonatas authored and RussKie committed Oct 22, 2022
1 parent 4875988 commit 714d98a
Show file tree
Hide file tree
Showing 7 changed files with 215 additions and 29 deletions.
78 changes: 75 additions & 3 deletions GitCommands/Patches/PatchManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ public static class PatchManager
}
}

public static byte[]? GetSelectedLinesAsPatch(string text, int selectionPosition, int selectionLength, bool isIndex, Encoding fileContentEncoding, bool isNewFile)
public static byte[]? GetSelectedLinesAsPatch(string text, int selectionPosition, int selectionLength, bool isIndex, Encoding fileContentEncoding, bool reset, bool isNewFile, bool isRenamed)
{
var selectedChunks = GetSelectedChunks(text, selectionPosition, selectionLength, out var header);
IReadOnlyList<Chunk>? selectedChunks = GetSelectedChunks(text, selectionPosition, selectionLength, out string header);

if (selectedChunks is null || header is null)
{
Expand All @@ -43,9 +43,15 @@ public static class PatchManager
header = CorrectHeaderForNewFile(header);
}

// if file is renamed and selected lines are being reset from index then the patch undoes the rename too
if (isIndex && isRenamed && reset)
{
header = CorrectHeaderForRenamedFile(header);
}

string? body = ToIndexPatch(selectedChunks, isIndex, isWholeFile: false);

if (header is null || body is null)
if (body is null)
{
return null;
}
Expand Down Expand Up @@ -82,6 +88,72 @@ private static string CorrectHeaderForNewFile(string header)
return sb.ToString();
}

private static string CorrectHeaderForRenamedFile(string header)
{
// Expected input:
//
// diff --git a/original.txt b/original2.txt
// similarity index 88%
// rename from original.txt
// rename to original2.txt
// index 0e05069..d4029ea 100644
// --- a/original.txt
// +++ b/original2.txt

// Expected output:
//
// diff --git a/original2.txt b/original2.txt
// index 0e05069..d4029ea 100644
// --- a/original2.txt
// +++ b/original2.txt

string[] headerLines = header.Split(new[] { '\n' });
string? oldNameWithPrefix = null;
string? newName = null;
foreach (string line in headerLines)
{
if (line.StartsWith("+++"))
{
// Takes the "original2.txt" part from patch file line: +++ b/original2.txt
newName = line[6..];
}
else if (line.StartsWith("---"))
{
// Takes the "a/original.txt" part from patch file line: --- a/original.txt
oldNameWithPrefix = line[4..];
}
}

StringBuilder sb = new();

for (int i = 0; i < headerLines.Length; i++)
{
string line = headerLines[i];
if (line.StartsWith("--- "))
{
line = $"--- a/{newName}";
}
else if (line.Contains($" {oldNameWithPrefix} "))
{
line = line.Replace($" {oldNameWithPrefix} ", $" a/{newName} ");
}
else if (line.StartsWith("rename from ") || line.StartsWith("rename to ") || line.StartsWith("similarity index "))
{
// Note: this logic depends on git not localizing patch file format
continue;
}

if (i != 0)
{
sb.Append('\n');
}

sb.Append(line);
}

return sb.ToString();
}

public static byte[]? GetSelectedLinesAsNewPatch(GitModule module, string newFileName, string text, int selectionPosition, int selectionLength, Encoding fileContentEncoding, bool reset, byte[] filePreamble, string? treeGuid)
{
var selectedChunks = FromNewFile(module, text, selectionPosition, selectionLength, reset, filePreamble, fileContentEncoding);
Expand Down
2 changes: 2 additions & 0 deletions GitUI/CommandsDialogs/FormCommit.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3404,6 +3404,8 @@ internal TestAccessor(FormCommit formCommit)
internal CheckBox ResetAuthor => _formCommit.ResetAuthor;

internal CheckBox Amend => _formCommit.Amend;

internal void RescanChanges() => _formCommit.RescanChanges();
}
}

Expand Down
29 changes: 24 additions & 5 deletions GitUI/Editor/FileViewer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ private enum ViewMode
private Func<Task>? _deferShowFunc;
private readonly ContinuousScrollEventManager _continuousScrollEventManager;
private FileStatusItem? _viewItem;
private readonly TaskDialogPage _NO_TRANSLATE_resetSelectedLinesConfirmationDialog;

private static string[] _rangeDiffFullPrefixes = { " ", " ++", " + ", " +", " --", " - ", " -", " +-", " -+", " " };
private static string[] _combinedDiffFullPrefixes = { " ", "++", "+ ", " +", "--", "- ", " -" };
Expand Down Expand Up @@ -197,6 +198,16 @@ public FileViewer()

_fullPathResolver = new FullPathResolver(() => Module.WorkingDir);
SupportLinePatching = false;

_NO_TRANSLATE_resetSelectedLinesConfirmationDialog = new()
{
Text = TranslatedStrings.ResetSelectedLinesConfirmation,
Caption = TranslatedStrings.ResetChangesCaption,
Icon = TaskDialogIcon.Warning,
Buttons = { TaskDialogButton.Yes, TaskDialogButton.No },
DefaultButton = TaskDialogButton.Yes,
SizeToContent = true,
};
}

// Public properties
Expand Down Expand Up @@ -1446,7 +1457,9 @@ public void StageSelectedLines(bool stage)
GetSelectionLength(),
isIndex: !stage,
Encoding,
_viewItem.Item.IsNew);
reset: false,
_viewItem.Item.IsNew,
_viewItem.Item.IsRenamed);
}

if (patch is null || patch.Length == 0)
Expand Down Expand Up @@ -1476,8 +1489,7 @@ public void ResetNoncommittedSelectedLines()
return;
}

if (MessageBox.Show(this, TranslatedStrings.ResetSelectedLinesConfirmation, TranslatedStrings.ResetChangesCaption,
MessageBoxButtons.YesNo, MessageBoxIcon.Warning) == DialogResult.No)
if (TaskDialog.ShowDialog(Handle, _NO_TRANSLATE_resetSelectedLinesConfirmationDialog) == TaskDialogButton.No)
{
return;
}
Expand Down Expand Up @@ -1508,7 +1520,9 @@ public void ResetNoncommittedSelectedLines()
GetSelectionLength(),
isIndex: true,
Encoding,
_viewItem.Item.IsNew);
reset: true,
_viewItem.Item.IsNew,
_viewItem.Item.IsRenamed);
}
else
{
Expand Down Expand Up @@ -1579,7 +1593,9 @@ private void ApplySelectedLines(bool allFile, bool reverse)
selectionLength,
isIndex: false,
Encoding,
_viewItem.Item.IsNew);
reset: false,
_viewItem.Item.IsNew,
_viewItem.Item.IsRenamed);
}
else
{
Expand Down Expand Up @@ -1983,6 +1999,7 @@ public bool ShowSyntaxHighlightingInDiff
set => _fileViewer.ShowSyntaxHighlightingInDiff = value;
}

public TaskDialogPage ResetSelectedLinesConfirmationDialog => _fileViewer._NO_TRANSLATE_resetSelectedLinesConfirmationDialog;
public ToolStripButton IgnoreWhitespaceAtEolButton => _fileViewer.ignoreWhitespaceAtEol;
public ToolStripMenuItem IgnoreWhitespaceAtEolMenuItem => _fileViewer.ignoreWhitespaceAtEolToolStripMenuItem;

Expand All @@ -1992,6 +2009,8 @@ public bool ShowSyntaxHighlightingInDiff
public ToolStripButton IgnoreAllWhitespacesButton => _fileViewer.ignoreAllWhitespaces;
public ToolStripMenuItem IgnoreAllWhitespacesMenuItem => _fileViewer.ignoreAllWhitespaceChangesToolStripMenuItem;

internal CommandStatus ExecuteCommand(Command command) => _fileViewer.ExecuteCommand((int)command);

internal void IgnoreWhitespaceAtEolToolStripMenuItem_Click(object sender, EventArgs e) => _fileViewer.IgnoreWhitespaceAtEolToolStripMenuItem_Click(sender, e);
internal void IgnoreWhitespaceChangesToolStripMenuItemClick(object sender, EventArgs e) => _fileViewer.IgnoreWhitespaceChangesToolStripMenuItemClick(sender, e);
internal void IgnoreAllWhitespaceChangesToolStripMenuItem_Click(object sender, EventArgs e) => _fileViewer.IgnoreAllWhitespaceChangesToolStripMenuItem_Click(sender, e);
Expand Down
69 changes: 63 additions & 6 deletions IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using GitCommands;
using GitUI;
using GitUI.CommandsDialogs;
using GitUI.Editor;
using GitUI.UserControls;
using ICSharpCode.TextEditor;
using NUnit.Framework;

Expand Down Expand Up @@ -244,7 +246,7 @@ public void Should_stage_only_filtered_on_StageAll()
testform.StageAllToolItem.PerformClick();
var fileNotMatchedByFilterIsStillUnstaged = testform.UnstagedList.AllItems.Where(i => i.Item.Name == "file2.txt").Any();
var fileNotMatchedByFilterIsStillUnstaged = testform.UnstagedList.AllItems.Any(i => i.Item.Name == "file2.txt");
Assert.AreEqual(2, testform.StagedList.AllItemsCount);
Assert.AreEqual(1, testform.UnstagedList.AllItemsCount);
Expand Down Expand Up @@ -296,15 +298,15 @@ public void Should_unstage_only_filtered_on_UnstageAll()
testform.UnstageAllToolItem.PerformClick();
var fileNotMatchedByFilterIsStillStaged = testform.StagedList.AllItems.Where(i => i.Item.Name == "file2.txt").Any();
var fileNotMatchedByFilterIsStillStaged = testform.StagedList.AllItems.Any(i => i.Item.Name == "file2.txt");
Assert.AreEqual(2, testform.UnstagedList.AllItemsCount);
Assert.AreEqual(1, testform.StagedList.AllItemsCount);
Assert.IsTrue(fileNotMatchedByFilterIsStillStaged);
});
}

[Test, TestCaseSource(typeof(CommitMessageTestData), "TestCases")]
[Test, TestCaseSource(typeof(CommitMessageTestData), nameof(CommitMessageTestData.TestCases))]
public void AddSelectionToCommitMessage_shall_be_ignored_unless_diff_is_focused(
string message,
int selectionStart,
Expand All @@ -317,7 +319,7 @@ public void Should_unstage_only_filtered_on_UnstageAll()
expectedResult: false, expectedMessage: message, expectedSelectionStart: selectionStart);
}

[Test, TestCaseSource(typeof(CommitMessageTestData), "TestCases")]
[Test, TestCaseSource(typeof(CommitMessageTestData), nameof(CommitMessageTestData.TestCases))]
public void AddSelectionToCommitMessage_shall_be_ignored_if_no_difftext_is_selected(
string message,
int selectionStart,
Expand All @@ -330,7 +332,7 @@ public void Should_unstage_only_filtered_on_UnstageAll()
expectedResult: false, expectedMessage: message, expectedSelectionStart: selectionStart);
}

[Test, TestCaseSource(typeof(CommitMessageTestData), "TestCases")]
[Test, TestCaseSource(typeof(CommitMessageTestData), nameof(CommitMessageTestData.TestCases))]
public void AddSelectionToCommitMessage_shall_modify_the_commit_message(
string message,
int selectionStart,
Expand All @@ -344,7 +346,7 @@ public void Should_unstage_only_filtered_on_UnstageAll()
}

[Test]
public void editFileToolStripMenuItem_Click_no_selection_should_not_throw()
public void EditFileToolStripMenuItem_Click_no_selection_should_not_throw()
{
RunFormTest(async form =>
{
Expand Down Expand Up @@ -473,6 +475,61 @@ public void SelectedDiff_remembers_geometry()
});
}

[Test]
public void ShouldNotUndoRenameFileWhenResettingStagedLines()
{
RunFormTest(form =>
{
FormCommit.TestAccessor ta = form.GetTestAccessor();
FileViewer.TestAccessor selectedDiff = ta.SelectedDiff.GetTestAccessor();
FileViewerInternal? selectedDiffInternal = selectedDiff.FileViewerInternal;
// Commit a file, rename it and introduce a slight content change
string contents = "this\nhas\nmany\nlines\nthis\nhas\nmany\nlines\nthis\nhas\nmany\nlines?\n";
_referenceRepository.CreateCommit("commit", contents, "original.txt");
_referenceRepository.DeleteRepoFile("original.txt");
contents = contents.Replace("?", "!");
_referenceRepository.CreateRepoFile("original2.txt", contents);
ta.RescanChanges();
ThreadHelper.JoinPendingOperations();
ta.UnstagedList.SelectedItems = ta.UnstagedList.AllItems;
ta.UnstagedList.Focus();
ta.ExecuteCommand(FormCommit.Command.StageSelectedFile);
ta.StagedList.SelectedGitItem = ta.StagedList.AllItems.Single(i => i.Item.Name.Contains("original2.txt")).Item;
selectedDiffInternal.Focus();
ThreadHelper.JoinPendingOperations();
selectedDiffInternal.GetTestAccessor().TextEditor.ActiveTextAreaControl.SelectionManager.SetSelection(
new TextLocation(2, 11), new TextLocation(5, 12));
int textLengthBeforeReset = selectedDiffInternal.GetTestAccessor().TextEditor.ActiveTextAreaControl.Document.TextLength;
selectedDiff.ResetSelectedLinesConfirmationDialog.Created += (s, e) =>
{
// Auto-press `Yes`
selectedDiff.ResetSelectedLinesConfirmationDialog.Buttons[0].PerformClick();
};
selectedDiff.ExecuteCommand(FileViewer.Command.ResetLines);
ta.RescanChanges();
ThreadHelper.JoinPendingOperations();
int textLengthAfterReset = selectedDiffInternal.GetTestAccessor().TextEditor.ActiveTextAreaControl.Document.TextLength;
textLengthBeforeReset.Should().BeGreaterThan(0);
textLengthAfterReset.Should().BeGreaterThan(0);
textLengthAfterReset.Should().BeLessThan(textLengthBeforeReset);
FileStatusItem? stagedAndRenamed = ta.StagedList.AllItems.FirstOrDefault(i => i.Item.Name.Contains("original2.txt"));
stagedAndRenamed.Should().NotBeNull();
stagedAndRenamed!.Item.IsRenamed.Should().BeTrue();
});
}

private void RunGeometryMemoryTest(Func<FormCommit, Rectangle> boundsAccessor, Action<Rectangle, Rectangle> testDriver)
{
var bounds1 = Rectangle.Empty;
Expand Down
17 changes: 17 additions & 0 deletions IntegrationTests/UI.IntegrationTests/GlobalSetupOnce.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using NUnit.Framework;

[SetUpFixture]
public class GlobalSetupOnce
{
[OneTimeSetUp]
public void RunBeforeAnyTests()
{
Application.EnableVisualStyles();
Application.SetCompatibleTextRenderingDefault(false);
}

[OneTimeTearDown]
public void RunAfterAnyTests()
{
}
}
17 changes: 17 additions & 0 deletions UnitTests/CommonTestUtils/GitModuleTestHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,23 @@ public string CreateRepoFile(string fileName, string fileContent)
return CreateRepoFile("", fileName, fileContent);
}

public string DeleteRepoFile(string fileName)
{
string parentDir = Module.WorkingDir;
string filePath = Path.Combine(parentDir, fileName);
if (!Directory.Exists(parentDir))
{
return filePath;
}

if (File.Exists(filePath))
{
File.Delete(filePath);
}

return filePath;
}

/// <summary>
/// Set dummy user and email locally for the module, no global setting in AppVeyor
/// Must also be set on the submodule, local settings are not included when adding it
Expand Down

0 comments on commit 714d98a

Please sign in to comment.