FullPathResolver: Avoid exception for illegal characters

[gerhardol]

Fixes #8571
Fixes #8364
Fixes #8437
Fixes #7862

Proposed changes

FullPathResolver throws if a file name contains Windows illegal characters that could be OK in Git repos.
As FullPathResolver is often used to check if a local file exists for instance to enable menu items, an exception
is normally not required and null can be returned instead.
For instance File.Exists(null) ==false.

In some situations returning null will still cause a null exceptions instead.
Reviewed the usage of Resolve() to change situations where null should not be fatal. This review is best effort, there may be other situations that should be changed too. Most "null unhandled" are used in Process(), there are other PRs there.
Some handling already use try-catch, like Delete in FormCommit.

An alternative would be to add a new method like bool TryResolve(string path) or bool Resolve(string path, bool throw = true) and change the about 45 of 60 Resolve() call sites to explicitly not throw.

Test methodology

Tests updated

---

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

[codecov]

Codecov Report

Merging #8575 (cc48144) into master (45ffd65) will decrease coverage by 0.00%.
The diff coverage is 44.31%.

@@            Coverage Diff             @@
##           master    #8575      +/-   ##
==========================================
- Coverage   55.13%   55.12%   -0.01%     
==========================================
  Files         904      904              
  Lines       65466    65474       +8     
  Branches    11853    11857       +4     
==========================================
- Hits        36094    36093       -1     
- Misses      26548    26554       +6     
- Partials     2824     2827       +3     

Flag	Coverage Δ
production	42.12% <37.17%> (-0.03%)	⬇️
tests	94.91% <100.00%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[RussKie]

LGTM, though once we move to .NET most of this will be irrelevant. In .NET Core 2.1 System.IO has gone an extensive update, and a number of exceptions that were previously thrown aren't thrown any more.
Here's a blog post https://docs.microsoft.com/en-us/archive/blogs/jeremykuhne/system-io-in-net-core-2-1-sneak-peek

[gerhardol]

LGTM, though once we move to .NET most of this will be irrelevant. In .NET Core 2.1 System.IO has gone an extensive update, and a number of exceptions that were previously thrown aren't thrown any more.
Here's a blog post https://docs.microsoft.com/en-us/archive/blogs/jeremykuhne/system-io-in-net-core-2-1-sneak-peek

Many of the changes in the second commit are still needed with Core 2.1.

I added one new commit, to be able to show the file in diff tab.
A repo to test with: git@github.com:gerhardol/tmp_test.git

RevTree does not work still, will be handled by the .NET Core 2.1 changes (some Path methods that throws when getting extensions etc. Should this be fixed here?)

[gerhardol]

Some corrections and more situations handled.
I added a wrapper for Path.Combine() and Path.GetExtension() to handle Git manipulation with Path utilities, that could be replaced with .NET Core if the app is migrated.

It is now possible to view commits with illegal characters in the path, also in the revtree.
Such commits cannot be checked out in Git, so you cannot do much more than than to view in RevDiff and RevTree.
Also difftool cannot be opened. Most context menu items are as expected, but Git fails for instance to run difftool (also from command line). Some details could be improved, like you can "Save for diff later" also if the path is evaluated as empty, but that are edge cases.

There has been a number of similar cases in the past, this should prevent some to occur.

LGTM, though once we move to .NET most of this will be irrelevant. In .NET Core 2.1 System.IO has gone an extensive update, and a number of exceptions that were previously thrown aren't thrown any more.
Here's a blog post https://docs.microsoft.com/en-us/archive/blogs/jeremykuhne/system-io-in-net-core-2-1-sneak-peek

Note that the official API is still not updated, the blog post is the best source I found to describe the change.
I am a little surprised that there are so few mentions about this on the net...
The MS open issue about fixing the documentation:
dotnet/dotnet-api-docs#1685

[RussKie]

Note that the official API is still not updated, the blog post is the best source I found to describe the change.
I am a little surprised that there are so few mentions about this on the net...

Aye! I had to ask questions internally to find this blog.

[RussKie]

Merge at will

[RussKie]

Please fix the history and merge

[gerhardol]

Please fix the history and merge

I have been a little busy and want ted to revise this again.
(We should not have so many open PRs in general.)

Some logic cleanups, and a resolve-stage revert
Unless there are protests, I will squash tomorrow and schedule a merge a day later.

[RussKie]

Just merge it. I think it has been open long enough for protests to surface :)