Add ddoc fullText to the CLI switches by wilzbach · Pull Request #7703 · dlang/dmd

wilzbach · 2018-01-14T06:00:24Z

Required for automatically generating the CLI options at dlang.org, see dlang/dlang.org#2068
There were the following differences:

-gc (only on dlang.org - the flag has been removed)
profile (not mentioned on dlang.org)
-run (not mentioned at the DMD output)
--version (not mentioned on dlang.org)

The DMD CLI wasn't alphabetical ordered for a few cases (https://dlang.org/dmd.html is).
(in the transition from the manual CLI output to the dynamically generated one, I kept the ordering to allow easy diffing of the output).

Generating the language transitions automatically depends on #7648
Anyways, the available language transitions aren't shown on dlang.org at the moment.

dlang-bot · 2018-01-14T06:00:25Z

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

wilzbach · 2018-01-14T06:02:34Z

src/dmd/cli.d

+                will not get called.)
+                $(LI `ModuleInfo` is not generated.)
+                $(LI $(LINK2 $(ROOT_DIR)phobos/object.html#.TypeInfo, `TypeInfo`)
+                instances will not be generated for structs.)


I'm not so sure that the CLI pages are the best place to document this. Imho we should just link to https://dlang.org/spec/betterc.html and improve this page if needed.

I agree, but let's leave that to another PR.

wilzbach · 2018-01-14T06:03:05Z

src/dmd/cli.d

        ),
        Option("c",
-            "do not link"
+            "compile only, do not link"


That's the text from dlang.org - imho a bit more descriptive.

wilzbach · 2018-01-14T06:05:16Z

src/dmd/cli.d

-            "generate documentation"
+            "generate documentation",
+            `$(P Generate $(LINK2 $(ROOT_DIR)spec/ddoc.html, documentation) from source.)
+            $(P Note: mind the $(LINK2 $(ROOT_DIR)spec/ddoc.html#security, security considerations).)


I don't think that the paragraphs are needed here, but it's a 1:1 from dlang.org

wilzbach · 2018-01-14T06:07:08Z

src/dmd/cli.d

        Option("inline",
-            "do function inlining"
+            "do function inlining",
+            `inline expand functions at the discretion of the compiler.


The "inline expand" looks out of place. I would just say "Inline functions"

I agree. Sorry, I didn't comment about that earlier. Care to make the change to "Inline functions..." before we merge?

jacob-carlborg · 2018-01-14T12:31:39Z

src/dmd/cli.d

+    string ddocText; /// Detailed description of the flag (in Ddoc)
+
+    ///
+    this(string flag, string helpText, TargetOS os = TargetOS.all)


Ideally all these parameters should be documented.

JinShil · 2018-01-14T13:42:16Z

I'm a little skeptical about this documentation being in the binary unless it actually serves some purpose for users of the compiler. At the moment I think this is only used as a datastore to keep all related information about the compiler flags in one place. That's perfectly fine, but I think we should version it out (may actually need string mixins) so it is only used for generating the documentation, but doesn't end up in the published binary.

wilzbach · 2018-01-14T18:58:12Z

I'm a little skeptical about this documentation being in the binary unless it actually serves some for users of the compiler.

It's needed for dlang/dlang.org#2068

but doesn't end up in the published binary.

That's why the CLI text is generated at CTFE, so these strings shouldn't end up in the binary.
I made this more explicit with version(DdocOptions) as I checked the binary and DMD didn't seem to be smart enough. It would be cool to avoid the need for version though, but apparently DMD judges a template instantiation at CTFE reson enough to send a templated function to object code.

WalterBright · 2018-01-14T19:18:08Z

That's why the CLI text is generated at CTFE, so these strings shouldn't end up in the binary.
I made this more explicit with version(DdocOptions) as I checked the binary and DMD didn't seem to be smart enough. It would be cool to avoid the need for version though, but apparently DMD judges a template instantiation at CTFE reson enough to send a templated function to object code.

Please file a bugzilla for this. But it still needs to be versioned out so it doesn't appear in the dmd binary, because updating the dmd that builds dmd may be a looong wait.

wilzbach · 2018-01-14T22:12:42Z

Please file a bugzilla for this.

https://issues.dlang.org/show_bug.cgi?id=18238

wilzbach · 2018-01-17T11:08:17Z

So as this is now using version(DdocOptions) and won't add up to the executable size. Is everyone OK with moving forward?
As mentioned the main benefit here is that there will be only one source of truth for the compiler arguments and dlang.org/dmd.html will automatically be in sync.

JinShil · 2018-01-17T12:17:56Z

Is everyone OK with moving forward?

LGTM

I don't see the output in DAutoTest. Please advise.

wilzbach · 2018-01-17T12:26:01Z

I don't see the output in DAutoTest. Please advise.

Yes, this PR is a dependency of dlang/dlang.org#2068
I could probably change dlang/dlang.org#2068 to use static if to softly fail if no ddoxText attribute is available, but unfortunately one of the two PRs has to be merged without the preview and I thought this PR here is easier to review, because it just adds text.

marler8997 · 2018-01-19T22:47:11Z

src/dmd/cli.d

+    {
+        string ddocText; /// Detailed description of the flag (in Ddoc)
+
+        this(string flag, string helpText, TargetOS os = TargetOS.all)


Maybe pull this method out of the version blocks since it appears to be the same in both cases?

marler8997 · 2018-01-19T22:49:25Z

src/dmd/cli.d

+            this.os = os;
+        }
+
+        this(string flag, string helpText, string ddocText, TargetOS os = TargetOS.all)


Maybe

version(DdocOptions) this.ddocText = ddocText;

Then remove the other instance of this constructor?

JinShil · 2018-01-20T08:45:53Z

@wilzbach This has been open for a week, which I think is ample time for others to weigh in. I'll probably merge this if you address @marler8997's comments above and no other objections arise.

wilzbach · 2018-01-20T08:57:51Z

Cool! I will do so once I am back and have access to my laptop.

wilzbach · 2018-01-24T07:18:22Z

(Rebased, addressed the review and resolved the merge conflict.)

wilzbach mentioned this pull request Jan 14, 2018

Generate CLI pages automatically dlang/dlang.org#2068

Merged

2 tasks

wilzbach commented Jan 14, 2018

View reviewed changes

wilzbach force-pushed the ddoc-cli branch from be61cf8 to 1a27836 Compare January 14, 2018 06:08

jacob-carlborg reviewed Jan 14, 2018

View reviewed changes

wilzbach added the Infrastructure:Continuous Integration label Jan 14, 2018

dlang-bot added the Review:Needs Rebase label Jan 16, 2018

wilzbach force-pushed the ddoc-cli branch from d9113c5 to 03d4a4e Compare January 17, 2018 00:42

dlang-bot removed the Review:Needs Rebase label Jan 17, 2018

JinShil approved these changes Jan 17, 2018

View reviewed changes

JinShil mentioned this pull request Jan 19, 2018

Document the new -i dmd compiler option dlang/dlang.org#2109

Closed

marler8997 reviewed Jan 19, 2018

View reviewed changes

marler8997 approved these changes Jan 19, 2018

View reviewed changes

wilzbach force-pushed the ddoc-cli branch 3 times, most recently from 67976f9 to 004438c Compare January 24, 2018 07:17

Add ddoc fullText to the CLI switches

ad5d69c

wilzbach force-pushed the ddoc-cli branch from 004438c to b83dc8f Compare January 24, 2018 12:35

ddocText: Start the first sentence with uppercase

6da2a05

s/Inline expand functions/Inline functions

b72d059

wilzbach force-pushed the ddoc-cli branch from 18c846d to b72d059 Compare January 24, 2018 12:53

JinShil added the Merge:auto-merge label Jan 25, 2018

dlang-bot merged commit 5814505 into dlang:master Jan 25, 2018

wilzbach mentioned this pull request Jan 25, 2018

Document new verbose option to redirect output to a file dlang/dlang.org#2107

Closed

wilzbach deleted the ddoc-cli branch January 29, 2018 12:09

Uh oh!

Conversation

wilzbach commented Jan 14, 2018

Uh oh!

dlang-bot commented Jan 14, 2018

Bugzilla references

Uh oh!

wilzbach Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil commented Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jan 14, 2018

Uh oh!

WalterBright commented Jan 14, 2018

Uh oh!

wilzbach commented Jan 14, 2018

Uh oh!

wilzbach commented Jan 17, 2018

Uh oh!

JinShil commented Jan 17, 2018

Uh oh!

wilzbach commented Jan 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil commented Jan 20, 2018

Uh oh!

wilzbach commented Jan 20, 2018

Uh oh!

wilzbach commented Jan 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wilzbach Jan 14, 2018 •

edited

Loading

JinShil commented Jan 14, 2018 •

edited

Loading