Skip to content

Comments

Add new default ddoc theme#6173

Merged
andralex merged 6 commits intodlang:masterfrom
jacob-carlborg:ddoc-new-theme
Oct 11, 2016
Merged

Add new default ddoc theme#6173
andralex merged 6 commits intodlang:masterfrom
jacob-carlborg:ddoc-new-theme

Conversation

@jacob-carlborg
Copy link
Contributor

@jacob-carlborg jacob-carlborg commented Oct 4, 2016

This adds a new default ddoc theme that generates a result with a design that can be used as is without manual styling. It outputs modern HTML 5, CSS 3 and passes the W3 HTML validator. This makes the out of the box experience a lot more pleasant.

The two first commits are from #6170.

Depends on #6170.

@PetarKirov
Copy link
Member

Nice work, @jacob-carlborg! Can you post a before and after screenshots for comparison?

@jacob-carlborg
Copy link
Contributor Author

Sure.

@jacob-carlborg
Copy link
Contributor Author

new_theme

old_theme

@jacob-carlborg jacob-carlborg changed the title Add new default ddoc theme [WIP] Add new default ddoc theme Oct 6, 2016
This allows better control of formatting of the output. For example,
it allows to output an unordered list instead of a description list.

By default it expands the first argument ($0) as is, basically making
the macro disappear, to keep backwards compatibility.
The second parameter ($1) of the DDOC_HEADER_ANCHOR macro expands to
the fully qualified name of the current symbol without the package and
module prefix. That is, for a module level symbol it's the name of a
symbol. For a nested symbol, like a method in a class, it's the symbol
name prefixed with the class name. The third parameter ($2) expands to
just the name of the current symbol.

The DDOC_MEMBER_HEADER wraps DDOC_HEADER_ANCHOR to allow a sensible
output format. The DDOC_MEMBER_HEADER is wrapped in the DDOC_MEMBER
macro.

By default these two macros expand to nothing, to keep backwards
compatibility.
This adds a new default ddoc theme that generates a result with a
design that can be used as is without manual styling. It outputs
modern HTML 5, CSS 3 and passes the W3 HTML validator. This makes the
out of the box experience a lot more pleasant.
@jacob-carlborg jacob-carlborg changed the title [WIP] Add new default ddoc theme Add new default ddoc theme Oct 11, 2016
@jacob-carlborg
Copy link
Contributor Author

@andralex ping

@andralex andralex merged commit 60ff2dc into dlang:master Oct 11, 2016
@andralex
Copy link
Member

Very nice. Thanks!

@jacob-carlborg
Copy link
Contributor Author

That was quick, thanks 😃.

@jacob-carlborg jacob-carlborg deleted the ddoc-new-theme branch October 12, 2016 06:42
@CyberShadow
Copy link
Member

CyberShadow commented Oct 12, 2016

This introduced some regressions in dlang.org (look at the doc autotester diffs for details). Because we auto-deploy website changes, the regressions have already propagated to the live website: http://dlang.org/phobos-prerelease/std_datetime.html#Month

Also, this made a massive impact on DDoc tests: test files that were previously under 100 lines are now over 1000 lines long. Perhaps there should be a way to opt-out of the new stylesheet, or the tests should be built with a simple .ddoc file that mimics the old default style.

Unless @jacob-carlborg can provide a timely dlang.org fix, I suggest that this is immediately reverted. @andralex

@andralex
Copy link
Member

If we preserve the existing ecosystem 100% by definition we cannot make progress. I'm okay with reverting the PR for now until we sort out the problems, but I'd rather put up with the larger test files rather than use a smaller mockup. What are the issues caused by the longer test files?

cc @jacob-carlborg

@CyberShadow
Copy link
Member

CyberShadow commented Oct 12, 2016

If we preserve the existing ecosystem 100% by definition we cannot make progress.

Yes, but breaking the live site could've been avoided by checking the CI systems in place to prevent exactly this. (Unfortunately there is no automated way that I know of to test if something "looks broken", so DDoc-related changes should be evaluated by at least a cursory examination of the diffs, available only two clicks away.)

I'm okay with reverting the PR for now until we sort out the problems

What I'm trying to say is that I think it would be better if the problems were noticed and sorted out before merging. :)

What are the issues caused by the longer test files?

That part was more of an observation in case it's a concern to someone, however one issue is that DDoc regressions (which the test files are supposed to help identify) will be more difficult to identify when the diffs are so big, that they are difficult to make sense of.

@jacob-carlborg
Copy link
Contributor Author

This introduced some regressions in dlang.org

@CyberShadow working on it. It should be easy to fix, it's one or two new macros that needs proper configuration.

Also, this made a massive impact on DDoc tests: test files that were previously under 100 lines are now over 1000 lines long

I suggested having DMD output a separate CSS file, but Andrei wanted a simper solution.

jacob-carlborg added a commit to jacob-carlborg/dlang.org that referenced this pull request Oct 12, 2016
The new Ddoc theme [1] introduced three new macros: DDOC_MEMBER,
DDOC_MEMBER_HEADER and DDOC_HEADER_ANCHOR. This tweak configures these
new macros to have no impact on the existing ddoc themes.

[1] dlang/dmd#6173
WalterWaldron added a commit to WalterWaldron/dmd that referenced this pull request Jan 1, 2017
dlang#6173 removed the ```__YEAR__``` tag from dlang#1699
@WalterWaldron WalterWaldron mentioned this pull request Jan 1, 2017
@MartinNowak
Copy link
Member

MartinNowak commented Jan 22, 2017

Unfortunately there is no automated way that I know of to test if something "looks broken", so DDoc-related changes should be evaluated by at least a cursory examination of the diffs, available only two clicks away.

You can render and visually diff, but that's rather expensive slow and requires acknowledgment for every pixel.
dlang/ddox#107

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants