Skip to content

Changes to better match $(D ...) in Phobos and bug fix #4306

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.

Sign up for GitHub

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

Jump to bottom
Merged
andralex merged 1 commit into from
Jan 25, 2015
Merged

Changes to better match $(D ...) in Phobos and bug fix #4306

andralex merged 1 commit into from
Jan 25, 2015

Conversation

@adamdruppe
Copy link
Contributor

@adamdruppe adamdruppe commented Jan 17, 2015

The existing $(D) macro does PSYMBOL highlighting, so change the highlightCode call to match that. std.datetime also revealed another off-by-one bug with embedded html so the i-- takes care of that. Test amended with these cases.

@adamdruppe adamdruppe mentioned this pull request Jan 17, 2015
Closed
@CyberShadow
Copy link
Member

CyberShadow commented Jan 17, 2015

Does this fix the true / <b>true</b> weirdness too?

@adamdruppe
Copy link
Contributor Author

adamdruppe commented Jan 17, 2015

No, that stays the same, and I think it should because the reason it gets bold is ddoc is seeing it as plain text. Code inside backticks is not interpreted as plain text anymore - it is minimally-highlighted code.

There's a very important difference there: <html> works, whereas plain text html is skipped and output in the final document unchanged. This whole thing is useless to me if I can't write literal html examples in the ddoc, so I won't change that.

The way this works is that there's nothing magical about $(D ...) (nor any other macro btw, all macros in ddoc are done by a separate macro.c file which knows nothing about D and nothing about ddoc context).

ddoc ALWAYS bolds the word true when it appears in plain text. Since the macro is non-magic, it counts as plain text, and gets bolded. (specifically, it wraps it in $(DDOC_KEYWORD) ... which is distinct from $(D_KEYWORD), which it uses in the code blocks).

Line 2096 of doc.c has the isKeyword function and you can see it only does it for true, false, and null. It is only called in one place: the default case of highlightText. Code highlighting is done by checking the keywords against dmd's lexer (line 2674 - tok.isKeyword() instead of isKeyword(), remember, no UFCS in c++!), but that's only triggered in side the --- example --- sections - the highlightCode2 function.

(which my first draft used and got keyword highlighting, but that wasn't universally liked)

/++
`true`
$(D true)
true
---
true
---

Macros:
D=<code>$0</code>
DDOC_BACKQUOTED = $(D $0)

+/

Makes:

<code>true</code> // quoted code, calls highlightCode() (called from highlightText)
<code><b>true</b></code> // macro, inherits <b> from highlightText
<b>true</b> // plain text, from highlightText
<pre class="d_code"><font color=blue>true</font> // block code, from highlightCode2 (which is called from highlightText)
</pre>

So, if you want true highlighted as a DDOC_KEYWORD, do NOT wrap it in the backquotes so it is treated as plain text by ddoc - whether passed to a macro or not is irrelevant. Or since the phobos seems to highlight inline code as bold anyway, it can stay the same and have no visual difference.

andralex
andralex reviewed Jan 18, 2015
View reviewed changes
src/doc.c Outdated
Copy link
Member

@andralex andralex Jan 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks misaligned

@adamdruppe
Copy link
Contributor Author

adamdruppe commented Jan 18, 2015

OOps, a couple tabs slipped in there. Fixed.

@kuettler
Copy link

kuettler commented Jan 21, 2015

LGTM, if I might say so. Any reason not to merge it?

@quickfur
Copy link
Member

quickfur commented Jan 24, 2015

LGTM

@andralex
Copy link
Member

andralex commented Jan 24, 2015

Auto-merge toggled on

@quickfur
Copy link
Member

quickfur commented Jan 24, 2015

Hooray!

andralex added a commit that referenced this pull request Jan 25, 2015
@andralex
Merge pull request #4306 from adamdruppe/backtick
8626695 
Changes to better match $(D ...) in Phobos and bug fix
@andralex andralex merged commit 8626695 into dlang:master Jan 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@adamdruppe @CyberShadow @kuettler @quickfur @andralex