From 714d98a87b40457fe654d23758e8397e16c98d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donatas=20Ma=C4=8Di=C5=ABnas?= Date: Thu, 20 Oct 2022 02:00:57 +0300 Subject: [PATCH] Fix resetting selected lines for renamed files in index (#10252) (cherry picked from commit 69a8570caa473aef419f9803e7bae683268cb42c) --- GitCommands/Patches/PatchManager.cs | 78 ++++++++++++++++++- GitUI/CommandsDialogs/FormCommit.cs | 2 + GitUI/Editor/FileViewer.cs | 29 +++++-- .../CommandsDialogs/FormCommitTests.cs | 69 ++++++++++++++-- .../UI.IntegrationTests/GlobalSetupOnce.cs | 17 ++++ .../CommonTestUtils/GitModuleTestHelper.cs | 17 ++++ .../CommonTestUtils/ReferenceRepository.cs | 32 ++++---- 7 files changed, 215 insertions(+), 29 deletions(-) create mode 100644 IntegrationTests/UI.IntegrationTests/GlobalSetupOnce.cs diff --git a/GitCommands/Patches/PatchManager.cs b/GitCommands/Patches/PatchManager.cs index 342387a6d38..81ec791ea90 100644 --- a/GitCommands/Patches/PatchManager.cs +++ b/GitCommands/Patches/PatchManager.cs @@ -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? selectedChunks = GetSelectedChunks(text, selectionPosition, selectionLength, out string header); if (selectedChunks is null || header is null) { @@ -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; } @@ -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); diff --git a/GitUI/CommandsDialogs/FormCommit.cs b/GitUI/CommandsDialogs/FormCommit.cs index dd8a56fe50d..e5b01ac7949 100644 --- a/GitUI/CommandsDialogs/FormCommit.cs +++ b/GitUI/CommandsDialogs/FormCommit.cs @@ -3404,6 +3404,8 @@ internal TestAccessor(FormCommit formCommit) internal CheckBox ResetAuthor => _formCommit.ResetAuthor; internal CheckBox Amend => _formCommit.Amend; + + internal void RescanChanges() => _formCommit.RescanChanges(); } } diff --git a/GitUI/Editor/FileViewer.cs b/GitUI/Editor/FileViewer.cs index 2e075fe71ab..25a5abcd9ed 100644 --- a/GitUI/Editor/FileViewer.cs +++ b/GitUI/Editor/FileViewer.cs @@ -73,6 +73,7 @@ private enum ViewMode private Func? _deferShowFunc; private readonly ContinuousScrollEventManager _continuousScrollEventManager; private FileStatusItem? _viewItem; + private readonly TaskDialogPage _NO_TRANSLATE_resetSelectedLinesConfirmationDialog; private static string[] _rangeDiffFullPrefixes = { " ", " ++", " + ", " +", " --", " - ", " -", " +-", " -+", " " }; private static string[] _combinedDiffFullPrefixes = { " ", "++", "+ ", " +", "--", "- ", " -" }; @@ -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 @@ -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) @@ -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; } @@ -1508,7 +1520,9 @@ public void ResetNoncommittedSelectedLines() GetSelectionLength(), isIndex: true, Encoding, - _viewItem.Item.IsNew); + reset: true, + _viewItem.Item.IsNew, + _viewItem.Item.IsRenamed); } else { @@ -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 { @@ -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; @@ -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); diff --git a/IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs b/IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs index ef43c4fa78a..ae4b019e058 100644 --- a/IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs +++ b/IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs @@ -4,6 +4,8 @@ using GitCommands; using GitUI; using GitUI.CommandsDialogs; +using GitUI.Editor; +using GitUI.UserControls; using ICSharpCode.TextEditor; using NUnit.Framework; @@ -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); @@ -296,7 +298,7 @@ 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); @@ -304,7 +306,7 @@ public void Should_unstage_only_filtered_on_UnstageAll() }); } - [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, @@ -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, @@ -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, @@ -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 => { @@ -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 boundsAccessor, Action testDriver) { var bounds1 = Rectangle.Empty; diff --git a/IntegrationTests/UI.IntegrationTests/GlobalSetupOnce.cs b/IntegrationTests/UI.IntegrationTests/GlobalSetupOnce.cs new file mode 100644 index 00000000000..adc4da0f53f --- /dev/null +++ b/IntegrationTests/UI.IntegrationTests/GlobalSetupOnce.cs @@ -0,0 +1,17 @@ +using NUnit.Framework; + +[SetUpFixture] +public class GlobalSetupOnce +{ + [OneTimeSetUp] + public void RunBeforeAnyTests() + { + Application.EnableVisualStyles(); + Application.SetCompatibleTextRenderingDefault(false); + } + + [OneTimeTearDown] + public void RunAfterAnyTests() + { + } +} diff --git a/UnitTests/CommonTestUtils/GitModuleTestHelper.cs b/UnitTests/CommonTestUtils/GitModuleTestHelper.cs index 577d2df08c3..e3fb4ba2ecc 100644 --- a/UnitTests/CommonTestUtils/GitModuleTestHelper.cs +++ b/UnitTests/CommonTestUtils/GitModuleTestHelper.cs @@ -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; + } + /// /// 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 diff --git a/UnitTests/CommonTestUtils/ReferenceRepository.cs b/UnitTests/CommonTestUtils/ReferenceRepository.cs index 0491a908842..5fc4c319c3e 100644 --- a/UnitTests/CommonTestUtils/ReferenceRepository.cs +++ b/UnitTests/CommonTestUtils/ReferenceRepository.cs @@ -8,13 +8,10 @@ namespace CommonTestUtils { public class ReferenceRepository : IDisposable { - private GitModuleTestHelper _moduleTestHelper; - private string _commitHash; + private readonly GitModuleTestHelper _moduleTestHelper = new(); public ReferenceRepository() { - _moduleTestHelper = new GitModuleTestHelper(); - CreateCommit("A commit message", "A"); } @@ -22,7 +19,7 @@ public ReferenceRepository() /// Reset the repo if possible, if it is null or reset throws create a new. /// /// The repo to reset, possibly null. - public static void ResetRepo(ref ReferenceRepository refRepo) + public static void ResetRepo(ref ReferenceRepository? refRepo) { if (refRepo is null) { @@ -45,7 +42,7 @@ public static void ResetRepo(ref ReferenceRepository refRepo) public GitModule Module => _moduleTestHelper.Module; - public string CommitHash => _commitHash; + public string? CommitHash { get; private set; } private const string _fileName = "A.txt"; @@ -71,26 +68,31 @@ public string CreateCommit(string commitMessage, string content = null) _moduleTestHelper.CreateRepoFile(_fileName, content ?? commitMessage); repository.Index.Add(_fileName); - _commitHash = Commit(repository, commitMessage); - Console.WriteLine($"Created commit: {_commitHash}, message: {commitMessage}"); - return _commitHash; + CommitHash = Commit(repository, commitMessage); + Console.WriteLine($"Created commit: {CommitHash}, message: {commitMessage}"); + return CommitHash; } - public string CreateCommit(string commitMessage, string content1, string fileName1, string content2, string fileName2) + public string CreateCommit(string commitMessage, string content1, string fileName1, string? content2 = null, string? fileName2 = null) { using LibGit2Sharp.Repository repository = new(_moduleTestHelper.Module.WorkingDir); _moduleTestHelper.CreateRepoFile(fileName1, content1); repository.Index.Add(fileName1); - _moduleTestHelper.CreateRepoFile(fileName2, content2); - repository.Index.Add(fileName2); + if (content2 != null && fileName2 != null) + { + _moduleTestHelper.CreateRepoFile(fileName2, content2); + repository.Index.Add(fileName2); + } - _commitHash = Commit(repository, commitMessage); - Console.WriteLine($"Created commit: {_commitHash}, message: {commitMessage}"); - return _commitHash; + CommitHash = Commit(repository, commitMessage); + Console.WriteLine($"Created commit: {CommitHash}, message: {commitMessage}"); + return CommitHash; } public string CreateRepoFile(string fileName, string fileContent) => _moduleTestHelper.CreateRepoFile(fileName, fileContent); + public string DeleteRepoFile(string fileName) => _moduleTestHelper.DeleteRepoFile(fileName); + public void CreateAnnotatedTag(string tagName, string commitHash, string message) { LibGit2Sharp.Signature author = new("GitUITests", "unittests@gitextensions.com", DateTimeOffset.Now);