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.

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

NBug: Include GE specific exception info #9564

Merged
merged 1 commit into from Oct 5, 2021

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Sep 9, 2021

Debug info for #9560 etc , needed after #9056

Proposed changes

When a Git command exits with errors, info about current directory, arguments etc is collected and presented in a popup.
This info is now also included in rthe NBug report.

Note: StandardError should be included as well as in Git Command Log, this a separate PR.

The popup is unchanged:

image

A cut exception below (as well as modified formatting), adding the three lines below ## Error Details

## Error Details

Command: C:\Program Files\Git\bin\git.exe
Arguments: -c color.ui=never -c diff.submodule=short -c diff.noprefix=false -c diff.mnemonicprefix=false -c diff.ignoreSubmodules=none -c core.safecrlf=false diff --find-renames --find-copies --name-status "07f7352af399f9da9923327abde3988d528c4205" "7caf16f7fcd2398b05b7d25de8e61d7205200094"
Working directory: C:\dev\gc\gitextensions_4\Externals\conemu-insidew\

GitExtUtils.ExternalOperationException: The directory name is invalid.
   at ExecutionResult GitCommands.GitModule.GetDiffFiles(string firstRevision, string secondRevision, bool noCache, bool nullSeparated) in C:/dev/gc/gitextensions_4/GitCommands/Git/GitModule.cs:line 2388
   at string ResourceManager.LocalizationHelpers.ProcessSubmoduleStatus(GitModule module, GitSubmoduleStatus status, bool moduleIsParent, bool limitOutput) in C:/dev/gc/gitextensions_4/ResourceManager/LocalizationHelpers.cs:line 289
   at async Task GitUI.GitUIExtensions.ViewChangesAsync(FileViewer fileViewer, FileStatusItem item, CancellationToken cancellationToken, string defaultText, Action openWithDiffTool)+GetSelectedPatchAsync(?) in C:/dev/gc/gitextensions_4/GitUI/GitUIExtensions.cs:line 146
   at async Task GitUI.GitUIExtensions.ViewChangesAsync(FileViewer fileViewer, FileStatusItem item, CancellationToken cancellationToken, string defaultText, Action openWithDiffTool) in C:/dev/gc/gitextensions_4/GitUI/GitUIExtensions.cs:line 94
   at async Task GitUI.CommandsDialogs.RevisionDiffControl.ShowSelectedFileDiffAsync() in C:/dev/gc/gitextensions_4/GitUI/CommandsDialogs/RevisionDiffControl.cs:line 479
   at async void GitUI.CommandsDialogs.RevisionDiffControl.DiffFiles_SelectedIndexChanged(object sender, EventArgs e)+(?) => { } in C:/dev/gc/gitextensions_4/GitUI/CommandsDialogs/RevisionDiffControl.cs:line 488
   at async void GitUI.ThreadHelper.FileAndForget(Task task, Func<Exception, bool> fileOnlyIf)+(?) => { } in C:/dev/gc/gitextensions_4/GitExtUtils/GitUI/ThreadHelper.cs:line 100

Test methodology

Tests adapted

Merge strategy

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


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

@ghost ghost assigned gerhardol Sep 9, 2021
@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Sep 14, 2021
@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Sep 14, 2021
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +357 to +360
//// Comment out to debug BugReporter
////catch (GitExtUtils.ExternalOperationException)
////{
////}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert back

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to have a comment with how to debug the BugReporter when the debugger is in use
What do you suggest?
Is the commented out code bothering you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping when squash merging in about a day unless clarified

@@ -22,7 +22,7 @@ public GitHubUrlBuilder(IErrorReportMarkDownBodyBuilder errorReportMarkDownBodyB
/// Generates a URL to create a new issue on GitHub.
/// </summary>
/// <see href="https://help.github.com/en/articles/about-automation-for-issues-and-pull-requests-with-query-parameters"/>
public string? Build(string url, SerializableException exception, string? environmentInfo, string? additionalInfo)
public string? Build(string url, SerializableException exception, string exceptionInfo, string? environmentInfo, string? additionalInfo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between exception and exceptionInfo and why the latter can't be derived from the former?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GE specific exceptions like GitExtUtils.ExternalOperationException are not available in BugReporter, so the summary for GE internal exceptions are passed as a string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this comment as xml-doc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Oct 3, 2021
@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Oct 3, 2021
@RussKie
Copy link
Member

RussKie commented Oct 3, 2021

Merge at will

* Open bug report in GitHub could fail
@gerhardol gerhardol merged commit 60dfae4 into gitextensions:master Oct 5, 2021
@ghost ghost added this to the 3.6 milestone Oct 5, 2021
@gerhardol gerhardol deleted the feature/nbug-git-info branch October 5, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants