-
Notifications
You must be signed in to change notification settings - Fork 63
[class-parse] support updated DroidDoc. #96
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
Conversation
With the latest Android N, Google has (again) changed API documentation format (i.e. doclet) and that broke API XML documentation parser. Unlike previous ones or Oracle Javadoc doclets, it is not easily matcheable with existing regex structure. So I made not a small set of changes to make it working. Unlike previous ones it takes a lot more time to process (e.g. more than minutes to process all API Level 24 docs), but better than broken. This is required to suppport any further API Levels (like 25). (ClassParse task has an issue that it never supported correct Javadoc processing as it ignores doclet differences. This only changes the default to droiddoc2 just to be able to process the latest doc. It needs different fix.)
| switch (DocletType) { | ||
| default: return new DroidDocScraper (dir); | ||
| default: return new DroidDoc2Scraper (dir); | ||
| case JavaDocletType.DroidDoc: return new DroidDocScraper (dir); |
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.
Shouldn't there also be a case JavaDocletType.DroidDoc2: entry, so that this can be explicitly selected?
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.
? Why? What is the point of "explicitly selected" ?
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.
The point is that the doc scrapers are specific, in that you can't reliably use the DroidDoc2Scraper with DroidDocScraper input, right?
So if someone has:
mono class-parse.exe --docspath=path/to/droiddoc1 foo.jar
Previously, they'll be using the DroidDocScraper. After this change, they'll be using the DroidDoc2Scraper, which won't support the previous docs, which may "break" things.
Consequently, if you can always explicitly specify the type -- and do so -- your binding won't break when we change the default scraper in the future:
mono class-parse.exe --docspath=path/to/droiddoc1 --docstype=droiddoc foo.jar
That said..., the <ClassParse/> MSBuild task has no way of specifying the ClassParse.DocletType property, which means the MSBuild task will always attempt to use the "first" entry, which is DroidDoc, despite the fact that ClassParse.DocumentationPaths can contain ~anything, as it's the concatenation of $(JavaDocPaths), $(Java7DocPaths), $(Java8DocPaths), and $(DroidDocPaths).
What presumably would be "best" is dropping the entire concept of ClassParse.DocletType, and instead have ClassParse.CreateDocScraper() "probe" the type of docs contained within dir so that we automatically use the appropriate scraper type.
However, we've mentioned this before and I believe that this "auto-probe" idea was considered to be difficult/impossible/flakey/inconsistent/etc., so I'm not entirely sure what's the best path forward for integration within Xamarin.Android.
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.
It has changed the default doc scraper from the old droiddoc to the new droiddoc. Who's using DroidDoc? Only ourselves. Unless someone is really enthusiastic and take sources from AOSP to generate documentation using droiddoc from there, no one has chance to use it in their own project. Thus we can change the default behavior with no hesitate.
ClassParse task not being able to differentiate doclet type is another issue that I believe we can fix, just like javadoc-to-mdoc alraedy implements. But it is much easier to make that build task to take the right doclet type and parse docs as expected. That can be done in much less cost and keeps compatibility with former behavior.
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.
That can be done in much less cost and keeps compatibility with former behavior.
Agreed.
Changes: dotnet/android-tools@f2af06f...26d65d9 * dotnet/android-tools@26d65d9: [Xamarin.Android.Tools.AndroidSdk] Update SDK component for API-30 (#99) * dotnet/android-tools@1878e43: [Xamarin.Android.Tools.AndroidSdk] Error & Warning Localization (#96) Additionally, the "main" branch for xamarin/xamarin-android-tools has changed from xamarin-android-tools/master to xamarin-android-tools/main. Update `.gitmodules` accordingly. Finally, update the `nuget restore Xamarin.Android.Tools.sln` command executed by `azure-pipelines.yaml` to also provide `-ConfigFile external\xamarin-android-tools\NuGet.config` in order to prevent the following failure on CI: NU1101: Unable to find package XliffTasks. No packages exist with this id in source(s): NuGetOrg
With the latest Android N, Google has (again) changed API documentation
format (i.e. doclet) and that broke API XML documentation parser.
Unlike previous ones or Oracle Javadoc doclets, it is not easily matcheable
with existing regex structure. So I made not a small set of changes
to make it working.
Unlike previous ones it takes a lot more time to process (e.g. more than
minutes to process all API Level 24 docs), but better than broken.
This is required to suppport any further API Levels (like 25).
(ClassParse task has an issue that it never supported correct Javadoc
processing as it ignores doclet differences. This only changes the
default to droiddoc2 just to be able to process the latest doc.
It needs different fix.)