Expanded one abbreviation in documentation #12405
ClosedPublic

Authored by syd on Jul 18 2016, 4:32 AM.

Details

Summary

Expanded one abbreviation in documentation Trac #12405

This is the first of potentially more commits on this task,
depending on whether the GHC devs actually like this kind
of contribution

Test Plan

Building documentation still works...

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
syd added a comment.Jul 19 2016, 7:27 AM
  • Fixed typos
syd marked 2 inline comments as done.Jul 19 2016, 7:28 AM
syd added inline comments.
compiler/hsSyn/HsBinds.hs
273

Bnitka told me this was Abstraction Bindings Export

syd updated this revision to Diff 8260.Jul 19 2016, 9:21 AM
syd edited edge metadata.
  • Expansions in basicTypes
bgamari accepted this revision.Jul 20 2016, 4:01 AM
bgamari edited edge metadata.

On the whole this looks great. The capitalization is a bit confusing: e.g. CafInfo is expanded to "Constant applicative form Information", where one might expect "Constant Applicative Form" to be capitalized as an abbreviation; I now see that you just follow the choice of abbreviation made in the declaration name. I suppose this seems sensible.

Anyways thanks for doing this. I've been wanting to clean up our Haddock documentation for quite some time. This is a very good start. Great work!

compiler/basicTypes/BasicTypes.hs
703

Out of curiosity why did the capitalization change here?

1195

It might be nice to include the "Used (instead of...)" discussion in the Haddock comment. In general I like seeing non-Haddock comments made into Haddock comments if they potentially offer value to API users (pure internal implementation details should probably remain non-Haddock comments, however).

compiler/basicTypes/DataCon.hs
496

While you are at it you may consider fixing the Haddock markup for these expressions, surrounding them in @...@ to ensure that they are rendered in a monospace typeface.

503–505

Spelling of "Implementation".

compiler/basicTypes/RdrName.hs
92–94

I still wish we had a note explaining the origin of the abbreviation "Rdr". Oh well, we'll have to leave this for another day.

compiler/basicTypes/Var.hs
391

This is Haddock markup, not Markdown; Use single-quotes here to make it a hyperlink to the identifier.

compiler/basicTypes/VarEnv.hs
421

"Coercion"

424

Spelling of "Coercion".

compiler/basicTypes/VarSet.hs
56–58

"Variable Set" is arguably less informative than the original form here. Admittedly the previous text can still be read on the next line, but I rather think that the first line of the comment should tell me what the thing *is*; often the expansion of the abbreviation can be a good proxy for this, but I'm not sure we should replace better documentation when it is available.

Perhaps this should just read "A non-deterministic Variable Set"? @thomie, what do you think?

compiler/hsSyn/HsBinds.hs
91–92

Perhaps "Haskell Value Bindings with separate Left and Right identifier types"?

855–857

I believe this should be "Specialisation".

857

Let's perhaps rewrite this as,

-- | Type checker Specialisation Pragmas
-- 
-- 'TcSpecPrags' conveys @SPECIALISE@ pragmas from the type checker to the desugarer
compiler/hsSyn/HsDecls.hs
458

Still needs fixing.

751

Again, type or class.

848

Let's mention "type" here as well,

Located type Family Result Signature
851

And here.

1878

I think it might be clearer if this were,

-- | Located Documentation comment Declaration
1913
-- | Warning pragma declarations
1922
-- | Warning pragma declaration
compiler/hsSyn/HsExpr.hs
147

Perhaps mention Arrows here.

685–690

It would be great if you could fix up the Haddock syntax here.

1093

Perhaps mention Arrow here.

1096

And here.

1194

Arrows

1431

Why isn't Side capitalized?

1520

Perhaps mention do here? On the other hand, people often think of do-syntax when you mention "statement". It's your call.

bgamari requested changes to this revision.Jul 20 2016, 4:03 AM
bgamari edited edge metadata.

Oops, I rather meant to request changes.

To clarify, I think most of the comments I made above would be nice improvements. If you disagree on particular cases then feel free to say so. Thanks again!

This revision now requires changes to proceed.Jul 20 2016, 4:03 AM
niteria added inline comments.
compiler/basicTypes/Var.hs
108

Coercion

compiler/basicTypes/VarSet.hs
56–58

I like "A non-deterministic Variable Set".

compiler/hsSyn/HsBinds.hs
121

No capitalization? In my opinion following the capitalization of the short form doesn't add much and makes the text look a bit chaotic. I think everyone can still see the connection even when they are capitalized normally.

273

Yes, this is based on:

| AbsBinds {                      -- Binds abstraction; TRANSLATION

above and the fact that it contains ABExport.

compiler/hsSyn/HsDoc.hs
21

Located

compiler/hsSyn/HsPat.hs
254–255

Coercion

261

Coercion

syd updated this revision to Diff 8282.Jul 20 2016, 9:00 AM
syd edited edge metadata.
syd marked 23 inline comments as done.
  • First round of comments.
syd added a comment.Jul 20 2016, 9:00 AM

These comments have been great.
Keep them coming :D

By the way, make html takes ages!

compiler/basicTypes/BasicTypes.hs
703

To match the declaration. identifier is not in OccInfo.

compiler/basicTypes/RdrName.hs
92–94
compiler/hsSyn/HsExpr.hs
147

Could you maybe tel me how? I want to make sure it's correct and I don't know the context here.

1093

Again: How?

1096

Again: How?

1194

Again: How?

1520

I just expanded the abbreviation... What should I mention?

bgamari added inline comments.Jul 20 2016, 11:23 AM
compiler/basicTypes/RdrName.hs
92–94

Ahh, I had never seen that! I see, "reader" like "the thing that reads the source". Thanks for pointing that out!

compiler/hsSyn/HsDecls.hs
458
-- | Located Declaration of a Type or Class

perhaps?

compiler/hsSyn/HsExpr.hs
1093

Perhaps,

-- | Located Haskell Command (e.g. a "statement" in an Arrow proc block)

Definitely a bit awkward, but I think it's at least a bit better. Feel free to iterate on it.

syd marked 3 inline comments as done.Jul 21 2016, 9:09 AM

Latest fixes also done.

Almost done, I think?

syd updated this revision to Diff 8298.Jul 21 2016, 9:10 AM
syd edited edge metadata.
  • Latest fixes

Just a few more comments. Thanks again for taking the time to do this!

compiler/hsSyn/HsExpr.hs
147

Perhaps,

-- | Command Syntax Table (for Arrow syntax)
1093

Here as well. The idea here is that we want to make it clear that "commands" are something specific to arrow-syntax. They are similar to a statement in a do block.

Nothing fancy is needed, just mention arrow syntax in the comments above.

1520

The idea here is similar to the Arrow business above; a "statement" is a construct particular to do syntax. Perhaps just say,

-- | Located @do@ block Statement
syd updated this revision to Diff 8306.Jul 22 2016, 4:38 AM
syd edited edge metadata.
  • last fixes?
syd added a comment.Jul 22 2016, 4:39 AM

This is getting slightly rediculous :D

~/arcanist/bin/arc diff --update D2406 HEAD^^^^^^^^^^^^
thomie requested changes to this revision.Jul 22 2016, 7:33 AM
thomie edited edge metadata.

Just some final remarks. After that I think we can push this (before any merge problems arise), and perhaps continue in a new Diff.

compiler/basicTypes/Var.hs
149
  • Type or Coercion Variable. I would skip the kind, since type=kind nowadays, and kind isn't mentioned in the abbreviation, but the or before Coercion is important.
  • Coercion with c
compiler/basicTypes/VarEnv.hs
421

Type or Coercion.

compiler/basicTypes/VarSet.hs
71

Coercion with c.

74
  • Type or Coercion
  • Coercion with c.
225
  • Type or Coercion.
  • Coercion with c.
This revision now requires changes to proceed.Jul 22 2016, 7:33 AM
syd marked 5 inline comments as done.Jul 22 2016, 10:47 AM

Next round done.

syd updated this revision to Diff 8317.Jul 22 2016, 10:47 AM
syd edited edge metadata.
  • last last fixes?
thomie accepted this revision.Jul 22 2016, 11:36 AM
thomie edited edge metadata.

Thanks!

This revision was automatically updated to reflect the committed changes.