Turn on -preview=markdown by dgileadi · Pull Request #2665 · dlang/dlang.org

dgileadi · 2019-06-22T20:29:16Z

This PR does three things:

Adds the -preview=markdown flag to the dlang.org Ddoc build.
Customizes the Ddoc-generated OTHER_CODE macro to support future instances of ``` some-language code fences.
Fixes places in the docs that were unintentionally interpreted as markdown.

The fixes in item 3 were generally for:

Places that match Markdown's asterisk rules for emphasis. These places are generally in code examples, but since they aren't wrapped in fenced code blocks Ddoc isn't aware that it's supposed to treat them as code.
Backslashes that aren't in code blocks, and so are treated as escaping the next character.
Lines that start with - or +, which in Markdown starts a list item.
Lines that start with >, which in Markdown starts a quote block.

dlang-bot · 2019-06-22T20:29:17Z

Thanks for your pull request and interest in making D better, @dgileadi! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

jacob-carlborg · 2019-06-23T10:28:11Z

articles/ctod.dd

       To specifiy the file c:\root\file.c you would use:
 $(CCODE
-char file[] = "c:\\root\\file.c";
+`char file[] = "c:\\root\\file.c";`


This will make the font bold. That will result in some code blocks look different from others. Not sure if that matters.

Once you pointed it out it bothered me enough that I went ahead and used ```c fences for these three items.

PetarKirov

Thanks for taking this closer to the finish line!
In addition to comments below, I also noticed that the output of some files changed like lex.dd, in which the list of escape sequences was now interpreted differently - IIRC from e.g. \n to n. In all those cases, the $(D ...) macro was used, so most likely the fix would be to use code blocks.

PetarKirov · 2019-06-24T06:26:36Z

articles/ctod.dd

@@ -859,8 +859,8 @@ $(H4 The C Way)
       C initializes arrays by positional dependence. C99 fixes the issue:
 $(CCODE


Can we used a fenced C code block here?

```C int a[3] = { 3,2,1 }; /* ... */ ```

Ditto for the rest of the $(CCODE ..) changes.

We certainly could, although we'd use a lowercase c to work with the existing CSS. The reason I didn't switch them is because I wanted to minimize the amount of changes, and if we change one we'd probably want to change them all.

articles/faq.dd

PetarKirov · 2019-06-24T06:32:14Z

changelog/2.074.0.dd

 $(UL
 $(LI $(LINK2 http://docs.algorithm.dlang.io/latest/mir_ndslice_topology.html, `mir.ndslice.topology`)
-        - Multidimensional `std.range` analog. Includes `bitwise`, `bitpack`, `zip`, `unzip`, `map`, `indexed` and many other features.)
+        \- Multidimensional `std.range` analog. Includes `bitwise`, `bitpack`, `zip`, `unzip`, `map`, `indexed` and many other features.)


Perhaps the dash should be moved to the end of the previous line to avoid escaping it?

changelog/2.075.1.dd

PetarKirov · 2019-06-24T06:36:22Z

changelog/2.079.0.dd

 $(P
 $(CONSOLE
-sed "s/delete \(.*\);/__delete(\1);/" -i **/*.d
+`sed "s/delete \(.*\);/__delete(\1);/" -i **/*.d`


Perhaps we should replace all uses of $(CONSOLE ...) with something along the lines of:

```console ```

Again, the reason I didn't make a change like this (and I was tempted to) is to minimize the amount of changes.

PetarKirov · 2019-06-24T06:40:06Z

changelog/index.dd

    var redir = window.location.href.replace(
-        /\/changelog\/index.html#(?:2\.|new2_)(\d+(?:\.\d+)?)$(DOLLAR)/,
+        /\\/changelog\\/index.html#(?:2\\.|new2_)(\\d+(?:\\.\\d+)?)$(DOLLAR)/,
        "/changelog/2.$(DOLLAR)1.html"


Wow, using inline js with regex inside ddoc files looks like a bad idea. All JS code should be defined in *.js files and included automatically inline if really necessary. That said, this out of the scope of your PR, so I'm fine with leaving it like this.

dgileadi · 2019-06-25T16:52:05Z

In addition to comments below, I also noticed that the output of some files changed like lex.dd, in which the list of escape sequences was now interpreted differently - IIRC from e.g. \n to n. In all those cases, the $(D ...) macro was used, so most likely the fix would be to use code blocks.

That worries me because I didn't see that behavior locally. It makes me wonder what else I might have missed. If it isn't too much trouble, could you let me know which files you saw that had issues?

CyberShadow · 2019-06-25T18:07:04Z

@ZombineDev The documentation tester isn't showing any changes in lex.html, so I'm also confused why you would be seeing a difference in your output.

There are changes in lex.verbatim, but this is a partial preview of the intermediary expanded state of macros, and changes there don't necessarily map to changes in the final HTML render.

PetarKirov · 2019-06-26T05:50:59Z

Yes, @CyberShadow, you're absolutely right! I must have confused the verbatim vs html diffs. @dgileadi sorry for the false alarm.

PetarKirov

I did another pass through your latest changes and I also checked the diff of all *.html files and I'm pleasantly surprised at how little has changed in the output. And were it changed I think it was an improvement.

Everything LGTM.

@dgileadi thanks for your outstanding work and persistence!

PetarKirov · 2019-06-26T06:02:32Z

@CyberShadow @aG0aep6G @wilzbach you guys may also want to have a look before we merge this PR. From my PoV it's good to go.

PetarKirov · 2019-06-26T06:07:40Z

Arghh.. I meant to leave this open for others to review. @CyberShadow if you have any high-level concerns that cannot be easily addressed with a follow-up, feel free to revert this PR, so we can have more time to review.

CyberShadow · 2019-06-26T15:43:16Z

No concerns, thank you @dgileadi for your work.

dgileadi requested review from CyberShadow, MartinNowak, WalterBright, andralex and wilzbach as code owners June 22, 2019 20:29

jacob-carlborg reviewed Jun 23, 2019

View reviewed changes

jacob-carlborg approved these changes Jun 23, 2019

View reviewed changes

PetarKirov reviewed Jun 24, 2019

View reviewed changes

Enable -preview=markdown

96f4ab3

dgileadi force-pushed the enable-markdown branch from a3b5a97 to 96f4ab3 Compare June 25, 2019 21:56

PetarKirov approved these changes Jun 26, 2019

View reviewed changes

PetarKirov added auto-merge Ddoc labels Jun 26, 2019

dlang-bot merged commit 7f257d7 into dlang:master Jun 26, 2019

PetarKirov added 72h no objection -> merge The PR will be merged if there are no objections raised. and removed auto-merge labels Jun 26, 2019

dgileadi deleted the enable-markdown branch June 26, 2019 19:28

wilzbach mentioned this pull request Aug 29, 2019

Update host compiler to 2.087 #2693

Merged

		@@ -859,8 +859,8 @@ $(H4 The C Way)
		C initializes arrays by positional dependence. C99 fixes the issue:
		$(CCODE

Uh oh!

Conversation

dgileadi commented Jun 22, 2019

Uh oh!

dlang-bot commented Jun 22, 2019

Bugzilla references

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PetarKirov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PetarKirov Jun 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgileadi commented Jun 25, 2019

Uh oh!

CyberShadow commented Jun 25, 2019

Uh oh!

PetarKirov commented Jun 26, 2019

Uh oh!

PetarKirov left a comment

Choose a reason for hiding this comment

Uh oh!

PetarKirov commented Jun 26, 2019

Uh oh!

PetarKirov commented Jun 26, 2019

Uh oh!

CyberShadow commented Jun 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PetarKirov Jun 24, 2019 •

edited

Loading