-
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: Refactor static site loading and directory structure #14378
Scaladoc: Refactor static site loading and directory structure #14378
Conversation
docs/sidebar.yml
Outdated
- title: Blog | ||
rootIndex: index.md | ||
pages: | ||
# - title: Blog |
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.
Let's remove this
@@ -34,9 +34,9 @@ trait Locations(using ctx: DocContext): | |||
cache.get(dri) match | |||
case null => | |||
val path = dri match | |||
case `docsRootDRI` => List("docs", "index") | |||
// case `docsRootDRI` => List("docs", "index") |
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.
Also leftover
@@ -82,34 +74,17 @@ abstract class Renderer(rootPackage: Member, val members: Map[DRI, Member], prot | |||
) | |||
updatedTemplates.result() | |||
|
|||
val newTemplates = updateSettings(templates, newSettings.to(ListBuffer)) | |||
val newTemplates = updateSettings(Seq(siteContext.staticSiteRoot.rootTemplate), newSettings.to(ListBuffer)) |
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 guess it was supposed to be Seq(rootTemplate)
or wh do we need property in line 43?
if files.size > 1 then | ||
val msg = s"ERROR: Multiple root index pages found: ${files.map(_.getAbsolutePath)}" | ||
report.error(msg) | ||
val relativizeFrom = if args.apiSubdirectory then docsPath else root.toPath |
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.
Nice trick :D
LoadedTemplate(withUpdatedTitle, List.empty, pathFromRoot.resolve(file.getName).toFile) | ||
case Sidebar.Root(_, _) => | ||
// Cannot happen | ||
??? |
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.
Maybe we could log it instead of throwing NotImplementedException
even though it will never hit this branch
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.
Or, should we define the type Sidebar
in a different way so that we wouldn’t even have to handle the Root
case here?
case "diff" => | ||
s"<pre><code class=\"language-diff hljs\" data-lang=\"diff\">${super.asString(nodes(1).render(context), context)}</code></pre>\n\n" | ||
case _ => | ||
report.warn("Unsupported highlight value. Currenlty supported values are: `diff`", file)(using ssctx.outerCtx) |
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.
Currently
typo
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.
Nice work @pikinier20!
There are several parts that I don’t understand in the diff, I have left some comments, do you mind clarifying?
Other than that, here are a few comments about the PR description:
I think that before merging we should also update documentation.
Sure! Thank you!
- We don't create sidebar.yml file and Scaladoc will load all documents that are present in /_docs/ directory and automatically generate navigation.
So, the navigation bar will mirror the hierarchy of directories and files in _docs
? What would be the order of the files? Alphabetical, I guess?
- We create sidebar.yml file (probably we also need to rename it?)
Why do you want to rename it?
(We can also add a option: hidden to render without entry in navigation)
What do you mean? I don’t understand this idea.
There's a setting
directory
that overrides this convention.
Could you please show an example?
With the -Yapi-subdirectory off (default), API documentation is in top-level and whole static site is inside _docs subdirectory. Otherwise, the static side goes to top-level and API is in api subdirectory.
Did you make any changes to -Yapi-subdirectory
in this PR? I can’t see them.
@@ -1,109 +0,0 @@ | |||
--- |
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.
How does the landing page look like now?
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.
This is a thing that we need to discuss. As I said in comment above - I don't think that custom landing pages are a responsibility of Scaladoc. For me, the entry point to the generated documentation should be standard doc page with the navigation etc. If we want to keep the "red" landing page on dotty.epfl.ch maybe we should just deploy it in the root directory and the entire generated nightly documentation would land into separate directory e.g. "reference" or "documentation"?
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.
Ah, so the landing page would be the one we currently see at https://dotty.epfl.ch/docs/index.html?
I realize now that the part that generates the sidebar can not be controlled by the users. There is a global layout that is applied to all the .md
files that don’t set a layout
in their front matter, is that right? However, that layout is not applied to .html
files such as this index.html
that you removed?
I don’t (yet) have a strong opinion about the idea of not supporting custom landing pages in Scaladoc.
This may have an impact on the way we could integrate the Scaladoc-generated website into the Jekyll-generated website.
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 don't really see any big advantage of generating custom landing page by Scaladoc. One can always do it manually. Usually, the links on landing page are not changed very often. Also, if we want to integrate Scaladoc and Jekyll, we can generate this page with Jekyll.
Alphabetical sounds good for me.
I just want to support multiple filenames because
The
Loading using YAML by default converts section title to the directory name that will contain the section. So for example: - title: Some section
subsection:
... would be rendered inside - title: Some section
directory: custom-directory
subsection:
... which results in rendering this section in
There's an added line in |
Do you mean that when there is a sidebar there is no way to produce a web page that is not linked from the sidebar? Or, it would be rendered only if we put |
Currently, it silently ignores the file - actually, it just never checks any file that is not declared in YAML. We can change that to scan the directory and warn about files that exist but are not present in config |
5af46b0
to
1e0ea84
Compare
d5d0433
to
e7c6003
Compare
Reading again the specification:
Does that overlap with the |
I think this is going in a good direction, could you add some user documentation? Most probably the best place to do so for now would be here: https://github.com/scala/docs.scala-lang/tree/main/_overviews/scala3-scaladoc |
If we specify a section without children, we get all pages that are in the directory and we add them all to navbar. - It's a shortcut to people that just want to have all pages without writing long YAML files. |
e7c6003
to
513088c
Compare
|
429f0d0
to
752f751
Compare
@julienrf I think I addressed all issues that you pointed out. We need there @BarkingBad to re-review the PR. In the meantime we can take a look at the entire work and apply some polishing if there's a need. EDIT: Also, in terms of documentation on docs.scala-lang, I think we can merge these two PRs asynchronously. Maybe it will be even simpler to first merge the PR here and then finish documentation because we'll know that nothing will be changed. |
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.
Nice!
Just to double-check, so how does the landing page look like now?
Right now, it's the main page of docs. |
…ctory to the new implementation
752f751
to
c5575b8
Compare
It looks to me that many URLs are broken by this change. For instance, https://dotty.epfl.ch/docs/usage/scaladoc/ is now located at https://dotty.epfl.ch/usage/scaladoc/. Would it be possible to put redirections in place? Also, it might be necessary to update links in the official Scala repositories (I can count 80 links in docs.scala-lang and 214 links in Dotty itself). |
My 2 cents on this is that the docs on dotty.epfl.ch are experimental (and updated every night). People should not expect much stability here. On the other hand, the stable docs can be found on docs.scala-lang.org (although they are a bit outdated now). We are currently working on a process to publish the docs on docs.scala-lang.org from the content of the dotty.epfl.ch repository, but this is not ready yet. |
@julienrf Thank you for your reply. So, what should be done in the meantime for all the broken links in the official Scala repositories? Would a pull request fixing them be welcome? |
Hey Nicolas, that’s really nice of you to propose your help! |
Static site in Scaladoc is probably one of its core features. Since it didn't have a lot of attention and also due to changing requirements, static site used to be not-so-clear piece of code. Having in mind upcoming changes to the Scaladoc, I decided to do a bigger refactor mainly in static site loading phase and also in directory structure.
I think that before merging we should also update documentation.
Static site loading
Previously, we loaded all files from filesystem and the
sidebar.yml
file was only used to generate LHS navigation. This solution was a bit problematic. Firstly, we could have pages that were only navigable by other pages or by sharing a link. It doesn't seem as a good thing because we could easily loose control over what's in the navigation and what is rendered. Also, it caused a problem with index pages: every section in navigation needs to have a index page otherwise we would get 404 after entering a section. Since thesidebar.yml
definition had nothing to do with location of files in filesystem, there was a problem with finding a location where the index should be rendered. (Navigation could e.g. aggregate files among more than one directory). This solution is also not flexible as we can't change the place where the file is rendered (Rendered directory structure is always the same as the source directory structure).I've changed that so we have two ways of loading static site:
sidebar.yml
file and Scaladoc will load all documents that are present in /_docs/ directory and automatically generate navigation.sidebar.yml
file (probably we also need to rename it?) which declares sections and pages that should be rendered and navigable. (We can also add a option: hidden to render without entry in navigation). The main advantage though is that the YAML pages structure defines the rendered directory structure. By default, for each section there will be directory created by converting its name to kebab case (e.g. Other New Features => other-new-features/). There's a settingdirectory
that overrides this convention. We can also define a section with title and index but without defining children which will cause loading documents from the directory of index file. e.g.will load all files from
some-section/
directory.Static site directory structure
After my changes, static site directory structure looks like this:
All documents should be in
_docs
directory. File-system loading crawls this directory and also every entry in YAML configuration is relative to this directory.Whole blog content should be in
_blog
directory. We check if the directory exists and based on that, we generate Blog section or not.All assets should be in
_assets
directory. The whole content of this directory is copied to the documentation and there's a check if asset exists by checking if the given path resolved in_assets
exists. e.g. forimages/logo.png
we check if_assets/images/logo.png
exists.This change makes a static site structure clearer and also simplifies a lot of logic in codebase.
The
-Yapi-subdirectory
settingThe setting was introduced to resolve possible conflicts between static site and API. However, we also decided to use it in order to define what is a top-level documentation. With the
-Yapi-subdirectory
off (default), API documentation is in top-level and whole static site is inside_docs
subdirectory. Otherwise, the static side goes to top-level and API is inapi
subdirectory.