Bug 15460 – Ddoc: merge the opening comment "<!-- Generated by Ddoc from filename.dd -->" into the default definition of DDOC
Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-12-18T16:08:49Z
Last change time
2022-10-24T13:26:52Z
Assigned to
No Owner
Creator
Andrei Alexandrescu
Comments
Comment #0 by andrei — 2015-12-18T16:08:49Z
In order to e.g. generate plaintext with Ddoc, the macro DDOC needs to be $(BODY) so there's no additional adornments. That's nice and simple. However, the annoying comment
<!-- Generated by Ddoc from filename.dd -->
is still emitted. There's no good reason to emit it separately - make it part of the DDOC default definition.
Comment #1 by destructionator — 2015-12-18T16:21:23Z
This is the code that does it:
buf.printf("$(DDOC_COMMENT Generated by Ddoc from %s)\n", m.srcfile.
toChars());
You could just define `DDOC_COMMENT=` to strip all comments from the generated code, since plain text doesn't have a comment facility anyway...
Comment #2 by andrei — 2015-12-18T16:29:33Z
(In reply to Adam D. Ruppe from comment #1)
> This is the code that does it:
>
> buf.printf("$(DDOC_COMMENT Generated by Ddoc from %s)\n", m.srcfile.
> toChars());
>
>
> You could just define `DDOC_COMMENT=` to strip all comments from the
> generated code, since plain text doesn't have a comment facility anyway...
Just do it!
Comment #3 by destructionator — 2015-12-18T16:30:47Z
$ make -f posix.mak
make -C src -f posix.mak
no cpu specified, assuming X86
make[1]: Entering directory `/home/me/d/pull-request-stuff/dmd/src'
make[1]: *** No rule to make target `lexer.h', needed by `s2ir.o'. Stop.
This pisses me off. A three minute change... but five minutes to get the git crap set up, and now it doesn't even work.
Unbelievable.
Comment #4 by destructionator — 2015-12-18T16:33:40Z
well looks like i had to make clean because I had stuff left over from when it was still C++ for last time I did a ddoc PR.
But still, ugh.
Comment #5 by andrei — 2015-12-18T16:35:19Z
(In reply to Adam D. Ruppe from comment #4)
> well looks like i had to make clean because I had stuff left over from when
> it was still C++ for last time I did a ddoc PR.
>
> But still, ugh.
I guess it's one of those things that are nobody's fault. Thanks for looking into the ddoc matter!
Comment #6 by destructionator — 2015-12-18T16:44:19Z
Comment #7 by destructionator — 2015-12-18T17:51:31Z
The auto tester cares about the exact bytes ddoc puts out?!?! Looks like it is failing because there's a space... but I didn't change the spaces... must be the change of new lines. This is ridiculous. Spaces don't even affect HTML.
I fear going through and improving the default macros because of these over specific tests. If I change a default macro to include a class name, the auto tester is going to fail it!
See, this is the big lie about low hanging fruit. A minor, obviously trivial change (just moving a useless comment!), means spending hours fighting with the tester too. If it was easy to use and catching legitimate bugs, I'd be OK with it... but it isn't easy to use (the Starting as a contributor page doesn't even *mention* dmd tests, and what it says about the Phobos one reveals yet another dmd makefile bug) and this isn't catching actual bugs. It is whining about spaces in HTML... which are ignored in HTML.
Friction like this just kills my momentum and spends time that I could have spent doing actually valuable stuff on chasing down an irrelevant whitespace "problem" that apparently appeared out of nowhere. (Seriously, look at the diff. The spaces were already there. Why is it only now complaining about it? But the tester failed before and is passing now so apparently it makes a difference. Illogical.)
Comment #8 by dlang-bugzilla — 2015-12-18T18:06:39Z
(In reply to Adam D. Ruppe from comment #7)
> [trim pointless rant]
Some of the tests are only there to make sure the output doesn't change by accident. If the change is intentional, update the tests to match.
Comment #9 by andrei — 2015-12-18T18:55:49Z
(In reply to Vladimir Panteleev from comment #8)
> (In reply to Adam D. Ruppe from comment #7)
> > [trim pointless rant]
>
> Some of the tests are only there to make sure the output doesn't change by
> accident. If the change is intentional, update the tests to match.
I agree with Vladimir sans the "pointless" part. Let's be nice to each other, y'all.
Comment #10 by destructionator — 2015-12-18T19:23:56Z
The autotester tests Ddoc by using diff to compare Ddoc's output with a "known good" file. If the output changes, then the diff says the files are different, and the autotester fails. A human is needed to check to see if the new output file is good or not. I don't know any other way to test it.
I suppose that other web frameworks don't have this problem because they don't check the output.
BTW, although you need dmd to generate Ddoc, you rarely need the latest version, because Ddoc rarely changes from version to version.
Comment #13 by dlang-bugzilla — 2015-12-19T02:38:03Z
(In reply to Andrei Alexandrescu from comment #2)
> (In reply to Adam D. Ruppe from comment #1)
> > This is the code that does it:
> >
> > buf.printf("$(DDOC_COMMENT Generated by Ddoc from %s)\n", m.srcfile.
> > toChars());
> >
> >
> > You could just define `DDOC_COMMENT=` to strip all comments from the
> > generated code, since plain text doesn't have a comment facility anyway...
>
> Just do it!
What does that mean?
Anyway, another option would be to move it its own separate macro, so that it can be disabled explicitly without affecting anything else.
Comment #14 by destructionator — 2015-12-19T03:23:10Z
Ali, the comment is still there, just moved a couple lines down, defined in the DDOC macro instead of above it. So if you want it you still have it.
BTW there's also a $(SRCFILE) macro you can use in your own code to output sucha comment yourself.
Comment #15 by destructionator — 2015-12-19T03:28:34Z
Walter:
"I suppose that other web frameworks don't have this problem because they don't check the output."
You check the output by looking for the relevant piece(s) for your test, not by considering any changed byte in the whole file to be a test failure.
This is the other thing I was considering writing about this weekend btw, writing meaningful tests. The parts of ddoc that are tricky are escaping, code highlighting, sections, macro expansion, recursive macro expansion, and the other few special features like _notEscaped and `code`. Checking that macro expansion correctly trims whitespace is a useful test too, but an independent once.
Comparing the actual definitions of the macros is a waste of everyone's time. Ddoc isn't broken because someone changed `B=<b>$0</b>` to be `B=<span style="font-weight: bold;>$0</span>` for example. You can argue that's silly, but it isn't broken - ddoc isn't supposed to care about exactly what the macros are defined to be.
Comment #16 by dlang-bugzilla — 2015-12-19T08:15:21Z
Again, a failing test doesn't necessarily mean that something's broken. Sometimes the test just needs to be updated, which can be done as easily as copying the test output over the versioned sample.
Comment #17 by razvan.nitu1305 — 2022-10-24T13:26:52Z