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

Use markdig for Markdown #1017

Merged
merged 14 commits into from Aug 1, 2017

Conversation

Projects
None yet
4 participants
@grokys
Contributor

grokys commented Jun 23, 2017

After submitting #1016 I looked into creating a markdown viewer based on markdig. There were the beginnings of WPF markdown support at https://github.com/grokys/markdig-wpf to which I submitted three pull requests to get markdown support to where we needed it.

Issues addressed:

  • #726 Markdown doesn't display code blocks.
  • #765 Markdown doesn't display checkboxes
  • #869 Markdown doesn't display tables
  • #870 Markdown doesn't strip html comments

To demonstrate the improvements you can open grokys/PullRequestSandbox#26. These are the results:

Before (master):

image

With fixes in #1016:

image

With this PR:

image

Downsides:

  • PRs to Markdig-wpf haven't yet been merged and there's not yet a nuget package for it, so another submodule :(
  • A package for markdig is available, but it's not signed. For the moment using StrongNameSigner to sign this, but I know that trying to use that has caused us problems in the past. In addition markdig is >=VS2017 so wouldn't be so easy to include as a submodule.

Update: Applied #1040 to remove Fody and now StrongNameSigner seems to be working without a problem, so submodules are no longer needed.

Depends on #1040

Fixes #726
Fixes #765
Fixes #869
Fixes #870

grokys added some commits Jun 23, 2017

Use MarkdownViewer from Markdig.Wpf.
Not yet working as markdig package dependency isn't strong name signed.

@grokys grokys requested review from shana, paladique and jcansdale Jun 23, 2017

@grokys grokys referenced this pull request Jun 23, 2017

Closed

Fix some problems with MarkdownViewer. #1016

3 of 3 tasks complete
@shana

This comment has been minimized.

Show comment
Hide comment
@shana

shana Jun 23, 2017

Collaborator

PRs to Markdig-wpf haven't yet been merged and there's not yet a nuget package for it, so another submodule :(

We have a myget feed for our own signed packages and we're also already putting packages in the lib directory, so we don't have to include it as a submodule 😄

Collaborator

shana commented Jun 23, 2017

PRs to Markdig-wpf haven't yet been merged and there's not yet a nuget package for it, so another submodule :(

We have a myget feed for our own signed packages and we're also already putting packages in the lib directory, so we don't have to include it as a submodule 😄

@shana

This comment has been minimized.

Show comment
Hide comment
@shana

shana Jun 23, 2017

Collaborator

Correction: I have a myget feed that I used at some point for some packages, but at the moment all the custom nuget packages we depend on (SQLitePCL.raw, Rx, EntryExitDecorator) are included in the lib directory in the repo.

Collaborator

shana commented Jun 23, 2017

Correction: I have a myget feed that I used at some point for some packages, but at the moment all the custom nuget packages we depend on (SQLitePCL.raw, Rx, EntryExitDecorator) are included in the lib directory in the repo.

@paladique

Looks good 👍 Is it possible to use the lib directory instead of the submodule?

@grokys

This comment has been minimized.

Show comment
Hide comment
@grokys

grokys Jul 10, 2017

Contributor

@paladique yes probably, however there's now a NuGet package for markdig-wpf with my changes. I'm hoping that with #1040 we can use StrongNameSigner and use that package directly.

Contributor

grokys commented Jul 10, 2017

@paladique yes probably, however there's now a NuGet package for markdig-wpf with my changes. I'm hoping that with #1040 we can use StrongNameSigner and use that package directly.

grokys added some commits Jul 11, 2017

Use StrongNameSigner for Markdig
Use StrongNameSigner for Markdig and Markdig.Wpf. This means the submodule has been removed.
@grokys

This comment has been minimized.

Show comment
Hide comment
@grokys

grokys Jul 11, 2017

Contributor

In a3cbd9a I use StrongNameSigner to remove the necessity of including the package as a submodule.

Contributor

grokys commented Jul 11, 2017

In a3cbd9a I use StrongNameSigner to remove the necessity of including the package as a submodule.

@jcansdale

jcansdale approved these changes Jul 12, 2017 edited

I'm glad it doesn't add an extra submodule! If this is the only repo that's using it, a NuGet package in the lib folder sounds like a plan (or better still the version distributed on NuGet). 👍

paladique and others added some commits Jul 13, 2017

Fix ncrunch build.
Need to copy StrongNameSigner to the ncrunch workspace.
@shana

shana approved these changes Aug 1, 2017

The targets file I think is in the wrong place and should be moved, otherwise LGTM!

Show outdated Hide outdated src/StrongNameSign.targets
<Message Text="Running StrongNameSigner on packages: @(StrongNameSignPackage)..." Importance="high"/>
<Exec ContinueOnError="false"
Command="&quot;$(MSBuildThisFileDirectory)..\packages\Brutal.Dev.StrongNameSigner.2.1.3\build\StrongNameSigner.Console.exe&quot; -in &quot;@(StrongNameSignPackage->'$(MSBuildThisFileDirectory)..\packages\%(Identity).*', '|')&quot;"/>
</Target>

This comment has been minimized.

@shana

shana Aug 1, 2017

Collaborator

Maybe put this next to the other targets files in the src/common folder, so they're all in one place?

@shana

shana Aug 1, 2017

Collaborator

Maybe put this next to the other targets files in the src/common folder, so they're all in one place?

This comment has been minimized.

@grokys

grokys Aug 1, 2017

Contributor

Good point - I didn't see the other targets there.

@grokys

grokys Aug 1, 2017

Contributor

Good point - I didn't see the other targets there.

@grokys grokys merged commit 76296ed into master Aug 1, 2017

5 checks passed

GitHub CLA @grokys has accepted the GitHub Contributor License Agreement.
Details
VisualStudio Build #7484846 succeeded in 92s
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins/build_log Jenkins Build Log
Details

@grokys grokys deleted the fixes/726-markdig branch Aug 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment