Improve recovery form #11753

pmiossec · 2024-05-25T10:17:12Z

Proposed changes

display type of data for blob in grid
perf: fetch ** dangling commit** metadata by batch (for my GE repo, for 1074 commits: 0.8s instead of 41.5s / only the long git fsck command --25s-- is now taking time. Overall: 1m5s before/ 25s after )
syntax highlighting also when opening in popup viewer
contextual shortcut are directly accessible (no need to open contextual menu first) & Ctrl-S added
support more file types & magic numbers
perf: don't do GUI costly processing twice

Before

After

Test methodology

Manual

Test environment(s)

Git Extensions 33.33.33
Build 3c8dd19 (Dirty)
Git 2.45.0.windows.1
Microsoft Windows NT 10.0.22631.0
.NET 8.0.1
DPI 96dpi (no scaling)
Portable: False

Merge strategy

I agree that the maintainer rebase merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

RussKie · 2024-05-25T15:04:03Z

src/app/GitUI/CommandsDialogs/FormVerify.LostObject.cs

@@ -78,6 +78,30 @@ private LostObject(LostObjectType objectType, string rawType, ObjectId objectId)
                ObjectId = objectId ?? throw new ArgumentNullException(nameof(objectId));
            }

+            public static string[] GetCommitsMetadata(IGitModule module, string commits)


How is this method used?

I have added a comment (and changed a little the interface)

RussKie · 2024-05-25T15:08:23Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

                UpdateFilteredLostObjects();
            }
        }

+        private bool _typeDetected = false;


Please keep fields at the top.
Initialisation is not required, it gets initialised implicitly.

mstv · 2024-05-25T20:01:38Z

src/app/GitUI/CommandsDialogs/FormVerify.LostObject.cs

@@ -32,9 +32,9 @@ private sealed partial class LostObject
            /// %s  - subject.
            /// %ct - committer date, UNIX timestamp (easy to parse format).
            /// </summary>
-            private static readonly string LogCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("log")
+            private static readonly string LogsCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("show")


Suggested change

private static readonly string LogsCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("show")

private static readonly string ShowCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("show")

src/app/GitUI/CommandsDialogs/FormVerify.LostObject.cs

mstv · 2024-05-25T20:07:54Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

@@ -26,9 +27,12 @@ public sealed partial class FormVerify : GitModuleForm

        private LostObject? _previewedItem;
        private string _defaultFilename = null;


Suggested change

private string _defaultFilename = null;

private string? _defaultFilename;

mstv · 2024-05-25T20:11:40Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

+                int currentBatch = 0;
+
+                while (currentBatch * batchSize < commits.Length)


Suggested change

int currentBatch = 0;

while (currentBatch * batchSize < commits.Length)

for (int currentBatch = 0; currentBatch * batchSize < commits.Length; ++currentBatch )

mstv · 2024-05-25T20:13:29Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

@@ -18,6 +18,7 @@ public sealed partial class FormVerify : GitModuleForm
        private readonly TranslationString _xTagsCreated = new("{0} Tags created." + Environment.NewLine + Environment.NewLine + "Do not forget to delete these tags when finished.");
        private readonly TranslationString _selectLostObjectsToRestoreMessage = new("Select objects to restore.");
        private readonly TranslationString _selectLostObjectsToRestoreCaption = new("Restore lost objects");
+        private readonly TranslationString _seems = new("seems");


Suggested change

private readonly TranslationString _seems = new("seems");

private readonly TranslationString _seems = new("seemingly");

mstv · 2024-05-25T20:14:32Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

+                        item.RawType += $" ({_seems.Text}: {GuessFileTypeWithContent(content)})";
+                    }
+
+                    await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();


Suggested change

await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

await this.SwitchToMainThreadAsync();

mstv · 2024-05-25T20:16:45Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

+        protected override bool ProcessCmdKey(ref Message msg, Keys keyData)
+        {
+            return mnuLostObjects.PreProcessMessage(ref msg) || base.ProcessCmdKey(ref msg, keyData);


Please add a comment why this is needed.
Doesn't have Form an option like KeyPreview?

Not an expert on this but your comment made me reevaluate.
I made some changes...

gerhardol

Have not run

mstv

Please avoid force push during pending reviews.

src/app/GitUI/CommandsDialogs/FormVerify.cs

mstv · 2024-05-25T21:01:08Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

+        protected override bool ProcessKeyPreview(ref Message msg)
+        {
+            // Check if keyboard shortcut is one provided by the contextual menu
+            return mnuLostObjects.PreProcessMessage(ref msg) || base.ProcessKeyPreview(ref msg);


Is there a more elegant way, @RussKie?

What's the bothering you here @mstv?
Looks good to me. I don't think this dialog sees particularly high use, so I don't think it's worth spending too much time to try to super-optimise it.

What's the bothering you here @mstv?

Because it is a workaround.

I don't think it's worth spending too much time to try to super-optimise it.

I agree.

I don't think this dialog sees particularly high use,

I wish everyone that this dialog will be the less used of the application but with this PR and the previous ones, GE is maybe now the best Git GUI to recover lost commits and especially mistakenly deleted staged files.

I'm afraid your thought was lost in translation, sorry... though I think I got the idea that this dialog could be useful in recovering accidentally discarded stages files. If this is correct, could you elaborate on this further please?

You have decrypted my bad English well to understand right.

In this screen, the checkbox "Show commits and tags" allows you to recover commits that are not in the reflog (and so that you can't retrieve easily with GE "Show all reflog references" or reflog features).

And "Show other objects" allows you to recover blobs that are not linked to a commit. And these blobs are contents that were added to the git objects database when you staged files (i.e. do a git add) but are no more pointed by a commit or the index.
So if you discard, by mistake, all the files after having staged these files, you are able to retrieve the contents (and only the content) with this recovery form.
Unfortunately, you can't retrieve the path of the files as the "tree" objects are still not stored in the git object database when you stage (they are stored in the .git\index file during the staging and will be stored only when you commit).
But the fact that now the form display the creation date of the blob ( #4882 ), try to determine the type of all the files based on there content ( #11727 and this PR) and display a content of the blob ( #8212 ) makes as convenient as possible to find the deleted content of the file you want to retrieve.

And I don't know a git GUI that provide such features that's why I said that maybe GE is the best tool to retrieve files in such case (before, as a last chance, having to try a recovery tool). Doing all that from the command line is also much more challenging.

The only missing feature is maybe the "search in blobs" feature but it should not be too difficult to do as everything is in place to be able to do it.

I think I will take some time later to add that to GE documentation ( or wiki?) later. At the moment, you could find some more explanation in stack overflow:

https://stackoverflow.com/a/58853981/717372

https://stackoverflow.com/a/22303923/717372

Wow. @pmiossec you're a legend

mstv

So this PR is ready for rebase in order to resolve the MC and for merging.

mstv · 2024-05-27T17:49:36Z

src/app/GitUI/CommandsDialogs/FormVerify.cs

+        protected override bool ProcessKeyPreview(ref Message msg)
+        {
+            // Check if keyboard shortcut is one provided by the contextual menu
+            return mnuLostObjects.PreProcessMessage(ref msg) || base.ProcessKeyPreview(ref msg);


What's the bothering you here @mstv?

Because it is a workaround.

I don't think it's worth spending too much time to try to super-optimise it.

I agree.

PR gitextensions#11753

for 1074 commits: 0.8s instead of 41.5s PR gitextensions#11753

PR gitextensions#11753

on double click PR gitextensions#11753

without needing to open contextual menu first PR gitextensions#11753

PR gitextensions#11753

PR #11753

for 1074 commits: 0.8s instead of 41.5s PR #11753

PR #11753

on double click PR #11753

without needing to open contextual menu first PR #11753

pmiossec force-pushed the improve_recovery branch from 78233dc to 167f698 Compare May 25, 2024 10:47

RussKie approved these changes May 25, 2024

View reviewed changes

pmiossec force-pushed the improve_recovery branch 3 times, most recently from 4778105 to d9df8d1 Compare May 25, 2024 19:57

mstv reviewed May 25, 2024

View reviewed changes

pmiossec force-pushed the improve_recovery branch from d9df8d1 to 4d02ad1 Compare May 25, 2024 20:39

gerhardol approved these changes May 25, 2024

View reviewed changes

mstv reviewed May 25, 2024

View reviewed changes

mstv approved these changes May 27, 2024

View reviewed changes

pmiossec added 7 commits May 27, 2024 21:50

feat(recovery): support more file types & magic numbers

7c33f5c

PR gitextensions#11753

perf(Recovery): don't do costly processing twice

da9e483

PR gitextensions#11753

perf(Recovery): fetch commit metadata by batch

e975c1f

for 1074 commits: 0.8s instead of 41.5s PR gitextensions#11753

feat(Recovery): display type of data for blob in grid

2ecdfde

PR gitextensions#11753

feat(Recovery): syntax highlighting also when opening in popup viewer

71e7fa4

on double click PR gitextensions#11753

fix(Recovery): shortcut are directly accessible

08b2e7d

without needing to open contextual menu first PR gitextensions#11753

feat(Recovery): sort objects to display more recents first

3ae292c

PR gitextensions#11753

pmiossec force-pushed the improve_recovery branch from 45615a5 to 3ae292c Compare May 27, 2024 19:53

pmiossec merged commit d261313 into gitextensions:master May 27, 2024

pmiossec added a commit that referenced this pull request May 27, 2024

feat(recovery): support more file types & magic numbers

c1fedfc

PR #11753

pmiossec added a commit that referenced this pull request May 27, 2024

perf(Recovery): don't do costly processing twice

73decce

PR #11753

pmiossec added a commit that referenced this pull request May 27, 2024

perf(Recovery): fetch commit metadata by batch

eccf199

for 1074 commits: 0.8s instead of 41.5s PR #11753

pmiossec added a commit that referenced this pull request May 27, 2024

feat(Recovery): display type of data for blob in grid

4e0c1b4

PR #11753

pmiossec added a commit that referenced this pull request May 27, 2024

feat(Recovery): syntax highlighting also when opening in popup viewer

9712bea

on double click PR #11753

pmiossec added a commit that referenced this pull request May 27, 2024

fix(Recovery): shortcut are directly accessible

c804c64

without needing to open contextual menu first PR #11753

pmiossec deleted the improve_recovery branch May 27, 2024 20:11

gerhardol added the release-notes-worthy label Jun 1, 2024

RussKie added this to the vNext milestone Jun 23, 2024

	private static readonly string LogsCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("show")
	private static readonly string ShowCommandArgumentsFormat = (ArgumentString)new GitArgumentBuilder("show")

		@@ -26,9 +27,12 @@ public sealed partial class FormVerify : GitModuleForm

		private LostObject? _previewedItem;
		private string _defaultFilename = null;

	private string _defaultFilename = null;
	private string? _defaultFilename;

		int currentBatch = 0;

		while (currentBatch * batchSize < commits.Length)

	private readonly TranslationString _seems = new("seems");
	private readonly TranslationString _seems = new("seemingly");

	await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
	await this.SwitchToMainThreadAsync();

Uh oh!

Improve recovery form #11753

Improve recovery form #11753

Uh oh!

Conversation

pmiossec commented May 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Before

After

Test methodology

Test environment(s)

Merge strategy

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerhardol left a comment

Choose a reason for hiding this comment

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmiossec May 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmiossec commented May 25, 2024 •

edited

Loading

pmiossec May 28, 2024 •

edited

Loading