-
Notifications
You must be signed in to change notification settings - Fork 865
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
chore: Fix exception thrown on TOC loading internally #9974
base: main
Are you sure you want to change the base?
chore: Fix exception thrown on TOC loading internally #9974
Conversation
16d74d0
to
76497b3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9974 +/- ##
==========================================
+ Coverage 74.31% 78.81% +4.49%
==========================================
Files 536 539 +3
Lines 23189 23440 +251
Branches 4056 4063 +7
==========================================
+ Hits 17234 18474 +1240
+ Misses 4853 3822 -1031
- Partials 1102 1144 +42 ☔ View full report in Codecov by Sentry. |
@@ -86,7 +86,45 @@ public static TocItemViewModel LoadSingleToc(string file) | |||
Logger.LogError(message, code: ErrorCodes.Toc.InvalidTocFile); | |||
throw new DocumentException(message, e); | |||
} | |||
} | |||
|
|||
private static bool IsListTocItems(StreamReader reader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit complicated. For handling YAML, I'm leaning towards we move to a future where YAML are parsed into a JSON node tree, and then deserialized to objects with standard JsonConverter.
Similar to https://github.com/dotnet/docfx/blob/v3/src/docfx/lib/json/YamlUtility.cs#L112 but with System.Text.Json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit complicated.
Remove stream relating unwind logics.
Instead, Read entire YAML text first, and then determine TOC type by YAML Parser.
76497b3
to
c5cffd3
Compare
c5cffd3
to
9d56541
Compare
280054a
to
323b0ce
Compare
This PR is intended to fix internal exception that occurred when deserializing
toc.yml
file.Current docfx implementation use
YamlDeserializerWithFallback
class to deserializetoc.yml
file.And following operation executed.
List<TocItemViewModel>
.TocItemViewModel
.This PR intended to fix following problems.
try-catch
logics are used when deserializingTocItemViewModel
asList<TocItemViewModel>
.-> Exceptions should not be thrown in the normal execution flow.
TocItemViewModel
. It requires to open safe file twice.