Skip to content

Enable DDoc Markdown by default#11654

Merged
dlang-bot merged 3 commits intodlang:masterfrom
wilzbach:markdown-default
Sep 3, 2020
Merged

Enable DDoc Markdown by default#11654
dlang-bot merged 3 commits intodlang:masterfrom
wilzbach:markdown-default

Conversation

@wilzbach
Copy link
Contributor

Markdown support has been in preview for more than a year and is used for the official docs (see e.g. #9645).
It's time to enable it by default. If folks encounter issues, there will be -revert=markdown for a while.

FYI @dgileadi

@dlang-bot
Copy link
Contributor

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.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11654"

Feature("dip25", "useDIP25",
"implement https://github.com/dlang/DIPs/blob/master/DIPs/archive/DIP25.md (Sealed references)", false, true),
// trigger deprecation message once D repositories don't use this flag anymore
Feature("markdown", "markdown", "enable Markdown replacements in Ddoc", false, false),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Druntime and Phobos are compiled with -de, so this must be done as a multi-stage PR iteration.

@wilzbach wilzbach force-pushed the markdown-default branch 4 times, most recently from 9227234 to 44d9d27 Compare August 30, 2020 17:51
@Geod24
Copy link
Member

Geod24 commented Aug 30, 2020

Nice! I was wondering today when should the other -preview switch be moved forward. Good to see some features being enabled.

@wilzbach
Copy link
Contributor Author

Nice! I was wondering today when should the other -preview switch be moved forward. Good to see some features being enabled.

Most -preview switches are problematic to enable, because they either break code or aren't fully finished or don't even compile druntime/Phobos, but this is one of the few that I think we can easily move ahead.

* {
* invs[0](), invs[1](), ...;
* }
* ---
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise it generates:

  <p class="para">
    Create inclusive invariant for struct/class by aggregating
 all the invariants in invs[].
      void __invariant() const [pure nothrow @trusted]
      {
          invs<a href="">0</a>, invs<a href="">1</a>, ...;
      }
  </p>

@wilzbach wilzbach added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Aug 30, 2020
@wilzbach
Copy link
Contributor Author

(Windows failure is unrelated - see #11653)

@@ -0,0 +1,3 @@
DDoc Markdown support has been enabled by default

If you encounter issues with this transition, use `-revert=markdown` and report an issue.
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I don't think this description is enough, because:

  • Let's not kid ourselves, most users don't use -preview. So re-stating some informations on it (where the documentation can be found, and an overview of the features / changes) is IMO important.
  • It's also important to have more than a one-liner for such an important feature. I know many users have asked for it over the year, and we should be a bit more proactive in marketing the good changes we're doing.
  • It's not a usual practice, so this is only a suggestion, but I would add a thank you note to @dgileadi for championing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I'm really lazy with changelog entries. I updated this one.

It's not a usual practice, so this is only a suggestion, but I would add a thank you note to @dgileadi for championing this.

Great idea! We should make this the usual practice ;-)

@Geod24
Copy link
Member

Geod24 commented Sep 3, 2020

ERROR: Newly generated header file (/home/circleci/dmd/generated/linux/release/64/frontend.h) doesn't match with the reference header file (/home/circleci/dmd/src/dmd/frontend.h)

DETAILS:

diff --git a/home/circleci/dmd/src/dmd/frontend.h b/home/circleci/dmd/generated/linux/release/64/frontend.h
index 172f7dae3..b707a27c4 100644
--- a/home/circleci/dmd/src/dmd/frontend.h
+++ b/home/circleci/dmd/generated/linux/release/64/frontend.h

Heh

@wilzbach
Copy link
Contributor Author

wilzbach commented Sep 3, 2020

ERROR: Newly generated header file (/home/circleci/dmd/generated/linux/release/64/frontend.h) doesn't match with the reference header file (/home/circleci/dmd/src/dmd/frontend.h)

Yeah, this is enforced now and should hopefully avoid silent C++ header breakage.
A bit silly for this PR as frontend.h can't be used yet and the current C++ headers don't provide a constructor for Params, but updating isn't hard (./build.d cxx-headers-test AUTO_UPDATE=1).

@Geod24 Geod24 added Merge:auto-merge and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels Sep 3, 2020
Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

Let's

@dlang-bot dlang-bot merged commit f0e381b into dlang:master Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants