XmlCommentMarkDownGenerator icon indicating copy to clipboard operation
XmlCommentMarkDownGenerator copied to clipboard

Added MSBuild Task to generate and merge markdown

Open egoughnour opened this issue 7 years ago • 15 comments

This change allows multiple input XML files and also merging of existing (manually maintained) markdown files. Added a test project for the MSBuild task. The AfterBuild target in .targets file is reduced to two steps:

  1. Warn if XML documentation isn't enabled.
  2. GenerateMarkdown task.

It also resolves #16. Tested with RhinoMocks and MSTest. Also tested the .nupkg file in VS.

egoughnour avatar Sep 12 '17 18:09 egoughnour

Resolves #12. Manually tested and in unit tests.

egoughnour avatar Sep 12 '17 20:09 egoughnour

Accidentally closed PR.

egoughnour avatar Sep 12 '17 20:09 egoughnour

Sorry, I've been off GH for a bit. This is really cool and is a lot to take in... if I'm reading this right, though, won't it break for existing users? If they don't have a readme.md in their project today it won't find it and it will error out.

Pxtl avatar Oct 25 '17 19:10 Pxtl

Yeah--it probably needs the default functionality to handle all existing use cases without failure and without reconfiguration. I'd have to look at it for a second to see whether that's what would happen.

egoughnour avatar Oct 26 '17 01:10 egoughnour

It looks like the only potential issue would be updating the NuGet package in projects where it is already installed. I am not sure what can be done in terms of detecting this situation (possibly in PowerShell). If there is a way to support detection of update (as opposed to clean install) then I would not mind to add some PowerShell to do it. (I am also unsure if this additional change should include pester tests along with whatever modules and scripts, but I am okay with making this happen.)

egoughnour avatar Oct 26 '17 05:10 egoughnour

I would be very tempted to make the "merge" functionality politely ignore the absense of the readme file. You'd still have the small breakage of existing users that their XmlDoc would now contain their Readme files, but you wouldn't have existing users seeing the project break for them.

I've been looking into how to make this configurable - there's not much documentation on Nuget build/.targets and build/.props files, but it looks like .props aren't user friendly and are a bit coarse.

I wouldn't want to include breaking functionality without an ability to turn it off.

Pxtl avatar Oct 26 '17 22:10 Pxtl

Suppressing some of the build tasks might work.
Forcing open release notes in a text file is possible with nuget. So maybe the key is to explain the on/off functionality in a small set of release notes (that the user can't avoid). Users could set a specific value of the CustomAfterMicrosoftCommonTargets property. This would be the path to another .proj or .props or .targets file included in the content of the nuget package but otherwise unused.

Turn that on and you get whatever functionality you want rather than the default.

Heck, there could be 3 or 4 alternative project files--supposing there are actually that many viable choices.

egoughnour avatar Oct 27 '17 00:10 egoughnour

And by a small set of notes I mean something like:

Don't like the default build output? Paste one of the following into your project file as an attribute under <MSBuild (or whichever tag would actually make sense).

  • Properties="CustomAfterMicrosoftCommonTargets=..\..\..\packages\<...>\OptionA.csproj" when generating Markdown, include only the files specified in OptionA.csproj

  • Properties="CustomAfterMicrosoftCommonTargets=..\..\..\packages\<...>\OptionB.proj" When Generating Markdown, exclude the .xml files in OptionB.proj or in the specified directory

  • Properties="CustomAfterMicrosoftCommonTargets=..\..\..\packages\<...>\OptionC.csproj" Some other combination of options.

egoughnour avatar Oct 27 '17 01:10 egoughnour

I'm not sure I'd want users to be forced to edit the .csproj file since it's such an opaque mess, but I know that's the way this is supposed to be done and the future of this stuff. It's too bad there's no way to apply XML comments at the Assembly level (only to classes/props/etc.) or I'd say put the config there - an assembly-level Readme.md would've been good.

It's tempting just to create a XmlCommentMarkdownGenerator.Config.xml (or json, as you like it) at the root of the target project and just keep settings there... but that seems less idiomatic than just editing the .csproj.

Or we could put YAML front-matter in the project's readme.md for parameters, Jekyll-style. It's pretty standard approach, but it would mean a bunch of YAML crap in the HTML view if you opened the readme.md

Like start the readme.md with

---
MergeXmlComments: true
AllowedCustomTags: all
---

And assume the user wants to not merge the readme if that block is missing.

Pxtl avatar Oct 27 '17 19:10 Pxtl

Okay. Using front-matter makes sense and it's a frequently used pattern. I'll add a commit or two to handle it.

egoughnour avatar Oct 27 '17 21:10 egoughnour

Bad News/Good News

Currently

  • [X] ~the exception handling isn't in place for the liquid templating~. Avoiding exception-related complexity with Environment.Exit(0) in the plugin itself.

  • [X] ~the actual call for markdown merging hasn't been added in the custom plugin/processor~ (and it won't be. see updated plan below.)

  • [X] the custom processing works fine

  • [X] the front matter is being read in testing

  • [ ] additional template processing can be added

  • [X] this has already been tested with Pretzel and Pretzel.ScriptCs.

  • [X] Pretzel is compatible with Jekyll.

  • [ ] It is simple to add pretzel into the nuget package and simply invoke it from the build.

egoughnour avatar Oct 29 '17 04:10 egoughnour

Using the Pretzel Adapter

  • [ ] Create WIX installer for adapter

  • [ ] Create Chocolatey package from installer and pretzel and pretzel.scriptcs dependencies

  • [ ] MSBuild task can either pull the front matter directly or the choco install can set up a pretzel instance for more complicated scenarios. This also enables forward compatibility once Jekyll or the WSL advances enough for a full auto Jekyll on windows nuget package to be feasible.

egoughnour avatar Oct 29 '17 18:10 egoughnour

Ooooof, can Pretzel is far too big of a dependency. I hate to keep poo-pooing this great work you're doing, but that's overkill. Detect the opening "---", then find the next line that starts with "---", and apply a YAML parser to everything in between to pull out the config fields.

Pxtl avatar Oct 30 '17 18:10 Pxtl

Alright. I’ll go with pull the front matter directly.

egoughnour avatar Oct 30 '17 19:10 egoughnour

Tested.

egoughnour avatar Oct 31 '17 13:10 egoughnour