-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scaladoc markdown preprocessor #13140
Scaladoc markdown preprocessor #13140
Conversation
scaladoc/src/dotty/tools/scaladoc/tasty/comments/markdown/SnippetFormattingExtension.scala
Show resolved
Hide resolved
2627f99
to
8ec44e0
Compare
db71769
to
8c43294
Compare
51698d2
to
bb13556
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not try to support both rendering the dotty website with scaladoc, and processing the md files to be rendered by Jekyll (on docs.scala-lang.org).
Instead, we should just focus on doing one thing: processing the md files that will be then rendered by Jekyll only. This will save us from introducing the postprocessing-layout
feature.
Reviewing the changes made to the md files is hard, I suggest the following process to be sure they are correct:
- update the markdown files in the docs.scala-lang repo to contain the fixes that have been applied in the dotty repo (the files in docs.scala-lang have been taken from commit 270bd8b, so we need to see what changes have been made to the
docs/
files since that commit and to backport them to the docs.scala-lang repo). - copy the files from docs.scala-lang to the dotty repo, without changing anything to them
scaladoc/src/dotty/tools/scaladoc/site/FrontMatterRenderer.scala
Outdated
Show resolved
Hide resolved
Thank you Julien for your patience to review this big chunk of code :D I will address all of your concerns in the threads. |
I agree
I was thinking about adding one more feature to the markdown front-matters, like file source. We can easily generate it because we know the |
Yes, that’s a good point. We should find a way to make this work. |
I'm thinking about opening 2 smaller PRs (one with css-fixes and one with markdown update) then would rebase this PR on top of the changes to reduce LOC of this PR and ease the whole process |
CSSes PR: #13335 |
Markdown backports to the docs.scala-lang repo: scala/docs.scala-lang#2160 |
Moved markdowns back to dotty: #13350 |
096472d
to
e3dfc31
Compare
66ce536
to
a3f7e1f
Compare
a3f7e1f
to
d453b0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did another pass where I added a few more comments. Please also note that former comments have not been addressed yet.
@@ -122,6 +122,16 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings: | |||
|
|||
val scastieConfiguration: Setting[String] = | |||
StringSetting("-scastie-configuration", "Scastie configuration", "Additional configuration passed to Scastie in code snippets", "") | |||
|
|||
val projectFormat: Setting[String] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use an enumeration type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is already ChoiceSetting
. It is hard to extends Setting
to be compatible with my custom type as I would have to modify doSet
method from dotty.tools.dotc.config.Settings
for my new enumeration ClassTag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. This is out of the scope of this PR, but we might need to allocate some resources at some point to clean the codebase. It looks very fragile now. If we add another format to the list we will have to manually look for all the places we pattern match on a format to check that we correctly handle it. If it was an enumeration the compiler would just give us exhaustivity warnings.
942362c
to
44fe92b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is urgent to move forward and clean the documentation authoring workflow. As long as we duplicate the markdown sources here and on docs.scala-lang, divergence will happen (see where we are in just a couple of months).
Hopefully, this work will help us maintain the documentation source files so that the content at docs.scala-lang.org will be easier to keep up to date, even for the compiler team.
By the way, I see that this PR changes some things on the source files that will be published to dotty.epfl.ch. I am not able to review those changes. I leave them up to you.
PoC of preprocessing markdown to get advantage of scaladoc linking and snippet compiler but preserve the flexibility to reuse docs in Jekyll or another templating engine.