Skip to content

Conversation

@adamdruppe
Copy link
Contributor

As I go through building my ddoc killer, I'm noticing a lot of deficiencies in the doc sources, some big, some small.

std.algorithm.searching had a typo, for example, leading to a broken link. std.base64 had a LINK2 title with commas in it, leading to cut off text.

Several of them have LINK2 modules, which breaks anywhere except /phobos (even ddox has these broken links).

Many of the examples don't say what they are trying to do or don't show enough information to actually be useful.

Functions don't refer to each other when they obviously could (obvious to someone who knows Phobos already that is, the new reader would have no idea). In general, browsing Phobos docs, it is easy to lose the forest for the trees. There's few examples of how to integrate modules.

There's an anti-pattern of two macros always appearing together, like $(M1 $(M2 ...)). That's silly, if M1 should always be applied to M2, just change M2's definition.

A great many FAQs in the support arena are not answered in the docs. I am adding a new section called "Diagnostics" with sample compiler error messages and showing how to fix them.

New users go to std.path to see what is in the current path, but that is actually in std.file. Let's have them link to each other.

Some functions create their own $(RED Important:) ... constructs. I propose changing them to be a well-defined semantic macro like $(PITFALL).

In a few places, I also add a blank line. This should be irrelevant to phobos, but it gives my doc generator a natural place to insert a table of contents after a brief introduction. The first line in ddoc is supposed to be a summary. Some functions lack this. Some write it with the assumption that it will be read immediately preceding the rest of the comment, making them read nonsensically if extracted into said table of contents. I reword a few of these.

I rearranged the toStringz overloads to put the one with the simpler signature first. Then a user browsing down will see that simple one as the entry point and the other one as an alternative. A small thing that has a bigger source diff than it really is, but makes things a bit more readable for the first-time browser.

I have a lot more in store. I'm willing to compromise a little to maintain compatibility with ddoc while still getting content enhancements merged, though my main goal here is to make better docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

line length

@CyberShadow
Copy link
Member

Please try to use more descriptive commit messages; none of the ones you wrote are very informative.

@CyberShadow
Copy link
Member

There's an anti-pattern of two macros always appearing together, like $(M1 $(M2 ...)). That's silly, if M1 should always be applied to M2, just change M2's definition.

I assume you mean LNAME and D? Not if the two are semantically distinct things - such as linking to things and marking what is D code. But even then, changing M2's definition is wrong on many levels, you should define a new marso instead.

Is LNAME/LNAME2 really used everywhere exclusively together with D?

@adamdruppe
Copy link
Contributor Author

On Thu, Dec 31, 2015 at 07:23:45PM -0800, Vladimir Panteleev wrote:

I assume you mean LNAME and D?

No, I don't even know what LNAME is. It doesn't show up in a grep either.

Oh there it is, in the druntime changelog...

But the one I had in mind was: $(D $(LREF appender))

If an LREF should always be rendered in code font, just make it
output a code font. And it shouldn't be always used, why is it
used here?

@CyberShadow
Copy link
Member

Yep, sorry, LREF not LNAME.

LREF is short for "local reference"... nothing about its name implies that its contents will be in code font. Do I really need to explain why changing the meaning of macros is bad?

@adamdruppe
Copy link
Contributor Author

On Thu, Dec 31, 2015 at 07:44:07PM -0800, Vladimir Panteleev wrote:

LREF is short for "local reference"... nothing about its name implies that its contents will be in code font.

Then why is it EVER in code font? On one of these pages, every
instance of it was wrapped in $(D). Why?

Do I really need to explain why changing the meaning of macros is bad?

This isn't the meaning, it seems to just be about a styling issue,
choosing a font.

@CyberShadow
Copy link
Member

Then why is it EVER in code font? On one of these pages, every instance of it was wrapped in $(D). Why?

I don't understand what you mean - clearly the answer is because every instance of it referred to a code identifier.

Anyway, I just looked at it (sorry I'm not home right now so I'm limited in doing such research), and didn't realize that LREF was never used on 'dlang.org`, only Phobos and always referring to symbols.

However, there are cases like: $(D GC.BlkAttr.$(LREF NO_MOVE))

To avoid regressions in HTML output, we would need to merge PRs introducing the LREF change simultaneously with all PRs in Phobos/Druntime that fix LREF references. It might be easier to do this by introducing a new macro first, then changing content to use that macro, then maybe removing the old macro. This also brings no confusion of what LREF means at any point in time.

@adamdruppe
Copy link
Contributor Author

On Thu, Dec 31, 2015 at 10:36:43PM -0800, Vladimir Panteleev wrote:

However, there are cases like: $(D GC.BlkAttr.$(LREF NO_MOVE))

That should just be $(LREF GC.BlkAttr.NO_MOVE) because the whole symbol is a local reference. Since DDOC_ANCHOR is defined to put a leading dot in front, LREF should do the same thing, so it continues to work on everything.

All those instances should be replaced. It was never actually correct to link to just one part of the name. If a global symbol was ever introduced with the same name, it would break that link!

To avoid regressions in HTML output, we would need to merge PRs introducing the LREF change simultaneously with all PRs in Phobos/Druntime that fix LREF references.

If it is a regression to add a html class to a link, this process is more broken than I realized.

This also brings no confusion of what LREF means at any point in time.

LREF means "local reference" and since you're documenting code, a local reference also happens to be a code identifier, so the css may style it as one.

@CyberShadow
Copy link
Member

That should just be $(LREF GC.BlkAttr.NO_MOVE) because the whole symbol is a local reference.

OK, just saying that such instances will need to be updated.

If it is a regression to add a html class to a link, this process is more broken than I realized.

Not sure what process you refer to, but I thought there's more to it than a CSS class.

Anyway, I just looked at LREF again and realized that it's already defined as <a href="#$1">$(D $1)</a>, so the $(D ...) is indeed redundant. Sorry for the noise.

@adamdruppe
Copy link
Contributor Author

On Fri, Jan 01, 2016 at 11:59:09AM -0800, Vladimir Panteleev wrote:

OK, just saying that such instances will need to be updated.

Right, I did a grep in Phobos and pushed the commit here too
(along with other things, I'm just editing docs as I see them
in the one branch)

@adamdruppe
Copy link
Contributor Author

So I'm continuing to push almost random changes here mostly just so you can see what I've been doing with my fork.

Between the pretty blue links in the beautifully formatted navigation bar telling me what I haven't looked at yet and the ease of just editing a file here without spending a lot of brain power on ddoc strangeness (most of this remains compatible but some of it isn't) and just not caring about the auto tester's irrelevant nitpicking, I'm able to make quite a few nice changes.

@dnadlinger
Copy link
Contributor

How do you plan to integrate your work back into upstream Phobos?

@wilzbach
Copy link
Contributor

wilzbach commented Mar 4, 2016

(ping) the longer you wait, the more conflicts you might get ;-)

@adamdruppe
Copy link
Contributor Author

I'm periodically pulling and rebasing to keep my changes up to date. However, there's mixes of ddoc compatible and non-ddoc changes in here now too.

@wilzbach
Copy link
Contributor

wilzbach commented Mar 4, 2016

Ah I thought you wanted to merge those typos and better descriptions. My fault :/

Is there an easy way to separate the compatible changes?

@adamdruppe
Copy link
Contributor Author

On Fri, Mar 04, 2016 at 12:12:34PM -0800, Seb wrote:

Is there an easy way to separate the compatible changes?

I think going through the diff line by line will do it, but I still
have a few other things to do.

@wilzbach
Copy link
Contributor

Several of them have LINK2 modules, which breaks anywhere except /phobos (even ddox has these broken links).

This is also addressed in #3926.

@adamdruppe
Copy link
Contributor Author

On Wed, Mar 30, 2016 at 01:19:04PM -0700, Seb wrote:

Several of them have LINK2 modules, which breaks anywhere except /phobos (even ddox has these broken links).

This is also addressed in #3926.

That's why I wrote the MREF macro!

@wilzbach
Copy link
Contributor

So more than a year went by & apparently no one wanted to merge this big PR. Moreover, thanks to a couple of deprecations (e.g. removal of XREF) there are quite a bunch of conflicts now.

@adamdruppe I am now closing this with regrets due to inactivity, but I highly encourage you to resubmit this work - maybe split into a PR per module?

@wilzbach wilzbach closed this Dec 26, 2016
@adamdruppe
Copy link
Contributor Author

adamdruppe commented Dec 26, 2016 via email

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