-
-
Notifications
You must be signed in to change notification settings - Fork 747
Markdownify code syntax from the old $(D code) macros to Markdown (i.e. code) [only two or more words]
#5185
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
Conversation
| $(T2 among, | ||
| Checks if a value is among a set of values, e.g. | ||
| $(D if (v.among(1, 2, 3)) // `v` is 1, 2 or 3)) | ||
| `if (v.among(1, 2, 3)) // `v` is 1, 2 or 3`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notice the backticks inside that...
| Note: | ||
| You may need to link to the $(B curl) library, e.g. by adding $(D "libs": ["curl"]) | ||
| You may need to link to the $(B curl) library, e.g. by adding `"libs": ["curl"]` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, this is a case where I don't think it should have been $(D ...) to begin with - this is json, not D, so it should have been using a generic code syntax. But you can't really automate a decision like that.
| Macros: | ||
| OBJECTREF=$(D $(LINK2 object.html#$0,$0)) | ||
| LREF=$(D $(LINK2 #.$0,$0)) | ||
| OBJECTREF=`$(LINK2 object.html#$0,$0)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also of the opinion that explicit links should not need to be wrapped in anything. If you find yourself always writing a(b(c)), then define d(c) => a(b(c)) and use d... which is what it is actually doing here, but it is repeated work on module level from one of the global macro definition files. I guess that's a separate issue from this PR, but these local ref macros should all be replaced with one of the generic ref things, that can style the text in the monospace font using a specific class name rather than wrapping the two macros together.
| $(BOOKTABLE , | ||
| $(TR $(TD $(D $(LREF InputRange))) | ||
| $(TR $(TD `$(LREF InputRange)`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this is a clear example of what I was talking about above: why wrap LREF in any kind of code macro, D or backtick, when LREF is already specifically referring to code? It should just output the appropriate thing to begin with and not have any wrapper.
| Ranges whose elements are sorted afford better efficiency with certain | ||
| operations. For this, the $(D $(LREF assumeSorted)) function can be used to | ||
| construct a $(D $(LREF SortedRange)) from a pre-sorted _range. The $(REF | ||
| operations. For this, the $(LREF `assumeSorted`) function can be used to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it shouldn't be
| UTF character support is restricted to | ||
| $(D '\u0000' <= character <= '\U0010FFFF'). | ||
| `'\u0000' <= character <= '\U0010FFFF'`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have different output than the before, since backtick will html encode its content in current D (both implementation and spec) and $(D ...) does not.
So the former will actually output <= character and the new will show, to the end user, as <= character. You can't use html entities or tags inside the backticks.
|
I just skimmed the diff and a few of my comments are that it was wrong before and wrong after, and a couple are that the translation is wrong and will cause minor regressions. Honestly though, I still fail to see the point of doing this translation at all and continue to see it as a net loss of useful information. This is why markdown has language specifiers you can put on code blocks... Moreover, doing it for for more than single word things lessens the effect of beauty... the macro syntax becomes less significant the more it wraps. So If, going forward, the backticks become popular for everything and |
|
@wilzbach @adamdruppe I'm okay with closing this if you are - sorry for the wasted work. |
Yeah you made an excellent case! And there are more important things in this world ;-) |
Alternative to #5183 where only two or more words within the code are replaced. Note that it also fixes the table in std.range.package from
$(D $(LREF foo))->$(LREFfoo`)CC @andralex @adamdruppe