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

Feature/Improved Tokens #7980

Merged
merged 8 commits into from
Jun 26, 2024

Conversation

chidozieononiwu
Copy link
Member

  • Switch C# APIView Parser from Flat list of Tokens to Tree Tokens

@chidozieononiwu chidozieononiwu requested a review from a team as a code owner March 29, 2024 20:32
@chidozieononiwu chidozieononiwu requested review from praveenkuttappan and maririos and removed request for a team March 29, 2024 20:33
@praveenkuttappan
Copy link
Member

Can you please include a markdown about token structure and token types so it will be easier for language owners to make the change?

@JonathanGiles
Copy link
Member

Can you please include a markdown about token structure and token types so it will be easier for language owners to make the change?

Yep - waiting on this so I can add Java support.

@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch 2 times, most recently from a935767 to dd99fc0 Compare April 17, 2024 22:27
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch from dd99fc0 to 512bcb3 Compare May 6, 2024 23:30
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch 2 times, most recently from 9d2a1fb to c663a95 Compare May 14, 2024 21:18
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch 2 times, most recently from 23bb386 to 3c3723e Compare May 31, 2024 15:56
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch 3 times, most recently from 4f5bb91 to bd103ea Compare June 3, 2024 12:56
Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Please beef up the documentation.

tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@praveenkuttappan praveenkuttappan left a comment

Choose a reason for hiding this comment

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

Parser name needs to be modified along with other small changes that can improve perf.

tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
return rootCommand.InvokeAsync(args).Result;


static void HandlePackageFileParsing(Stream stream, FileInfo packageFilePath, DirectoryInfo OutputDirectory, string outputFileName, bool runAnalysis)
Copy link
Member

Choose a reason for hiding this comment

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

This can potentially be simplified with the NuGet.Protocol package - samples here

src/dotnet/APIView/APIView/Model/TokenTreeModel.cs Outdated Show resolved Hide resolved
Url
```

### APITreeNode
Copy link
Member

Choose a reason for hiding this comment

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

Documenting here the classes won't scale and the documentation will get out of date. Instead, let's do the documentation in the class itself and point people there.

Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would rather not try to interpret C# code comments when working out how to write my parser in Java - I only fall back to looking there when the markdown is missing some details.

Copy link
Member

Choose a reason for hiding this comment

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

100% agree with @JonathanGiles. Parser authors should not be forced to dig around in C# source code as a substitute for actual documentation.

tools/apiview/parsers/CONTRIBUTING.md Outdated Show resolved Hide resolved
src/dotnet/APIView/APIView/Model/TokenTreeModel.cs Outdated Show resolved Hide resolved
src/dotnet/APIView/APIView/Model/TokenTreeModel.cs Outdated Show resolved Hide resolved
src/dotnet/APIView/APIView/Model/TokenTreeModel.cs Outdated Show resolved Hide resolved
src/dotnet/APIView/APIView/Model/TokenTreeModel.cs Outdated Show resolved Hide resolved
Parse C# Assemblly to Token Tree

Add Token Tree Documentation

Add LineDefinitionId and HideFromNavigation

Undo internals visible to setting

Sort NameSpaces and Types

Ensure Each node has Name and Id

Serialize to Message Pack

Add Tags to StructuredToken

update tree Token Parser Documentation

Attempt to Improve Serialized JSON Size

Properties for Serialization and Deserialization

Use shorter names for diagnostics

update APIView Parser Contributing guide

Delete TreeTokenCodeFile

Update APIView C# Parser Project Name

Add MessagePack Serialization

Switch to System.Json.Text working

Update Contributing Doc

Add Navigate to ID

Make Parser a dotnet Tool

Update Parser Version for CodeFile

App Parser Style Property

Output GZipped TokenFile

Switch to Human Readable Json Property Names, use compression

Fix some issues raised in pull request

Updating Contributing.md

Update Contributing Docs
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch from 5f2bed0 to daf0679 Compare June 19, 2024 03:08
chidozieononiwu and others added 3 commits June 19, 2024 12:14
Parse C# Assemblly to Token Tree

Add Token Tree Documentation

Add LineDefinitionId and HideFromNavigation

Undo internals visible to setting

Sort NameSpaces and Types

Ensure Each node has Name and Id

Serialize to Message Pack

Add Tags to StructuredToken

update tree Token Parser Documentation

Attempt to Improve Serialized JSON Size

Properties for Serialization and Deserialization

Use shorter names for diagnostics

update APIView Parser Contributing guide

Delete TreeTokenCodeFile

Update APIView C# Parser Project Name

Add MessagePack Serialization

Switch to System.Json.Text working

Update Contributing Doc

Add Navigate to ID

Make Parser a dotnet Tool

Update Parser Version for CodeFile

App Parser Style Property

Output GZipped TokenFile

Switch to Human Readable Json Property Names, use compression

Fix some issues raised in pull request

Updating Contributing.md

Update Contributing Docs

initial pass to documentation

fix

pr feedback

one missed
Initial pass to improve token documentation
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch from 1e228ab to 76ce86c Compare June 19, 2024 21:52
@chidozieononiwu chidozieononiwu force-pushed the feature/ImprovedTokens branch from 09f218f to 22a72db Compare June 26, 2024 18:25
@chidozieononiwu chidozieononiwu merged commit a6c219e into Azure:main Jun 26, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants