Skip to content

Conversation

@nathanaelbosch
Copy link
Contributor

@nathanaelbosch nathanaelbosch commented Dec 12, 2025

This PR implements the suggested adjustments from #1206, plus some additional minor things.

The major change is that code execution is now handled by Literate.jl, instead of Documenter.jl. This enables the functionality of this PR, to only run code of selected files, while including all other files without any code execution.

The logic to determine which scripts to execute is now the following:

  • If JL_FILES_TO_EXECUTE = :ALL then all files are run.
  • If JL_FILES_TO_EXECUTE is a list of filepaths, then only those files are executed, and all other ones are included without any code execution. This can be helpful to work on a particular file, or even just to iterate quickly when working on text or structure.
  • If the source file is older than the generated file, it is skipped altogether. This way, even when building the whole documentation to see the full up-to-date docs, one can save lots of time when working only on a subset of the documentation (or the structure).
    Overall I'd hope that this enables a much easier development workflow to work on the docs.

Additional changes:

  • Removed jupyter notebook builds: Previously Literate.jl was used both to generate markdown files and jupyter notebooks. If the latter are not actively used, removing them from the build chain can further speed up the process.
  • Added @debug statements: This is just something I found helpful while writing the PR
  • A comment to set draft=true for faster development: This seems to be one of these things that can sometimes be hard to find, but which are very helpful for people when they work on the documentation. I therefore added a small comment in makedocs such that people can find this setting quickly if they need it.
    I'm happy to keep or remove these as you prefer.

Things that are left to do:

  • Locally the documentation actually fails due to some missing references:
    Warning: Cannot resolve @ref for md"[Tutorial](@ref)" in src/guide/periodic_problems.md.
    
    I still have to check if this is an issue of this PR or if this is on master too.
  • .gitignore: I did not yet make sure that the .gitignore is aware of all the generated .md files that are now kept around. I'm not quite sure what the most elegant way is to solve this.
  • Should we set size_threshold_warn=nothing to reduce the number of warnings?
  • Checking if the CI builds the documentation correctly
  • New: Add some quick notes on how this mechanism works to the DFTK developer docs.

@nathanaelbosch nathanaelbosch marked this pull request as draft December 13, 2025 13:27
@nathanaelbosch nathanaelbosch marked this pull request as ready for review December 13, 2025 14:40
Copy link
Member

@mfherbst mfherbst left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the starting point. Let's see what CI says.

Comment on lines +201 to +206
@debug "Processing" file.src file.dest
# Skip all flies that did not change since last build
if mtime(file.src) <= mtime(file.dest) && !DEBUG
@debug "Skipping up-to-date file"
continue
end
Copy link
Member

Choose a reason for hiding this comment

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

Could handle debug printing more elegantly here by printing a different message depending on the mtime check.

file.src, file.dest;
flavor=Literate.DocumenterFlavor(),
credit=false,
preprocess=add_badges(badges),
Copy link
Member

Choose a reason for hiding this comment

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

The only reason to have the badges was to have the links to the binder platform for executing ipynb notebooks online. If we drop ipynb generation, we can drop the add_badges business, too.

end
end
end
# if !DEBUG
Copy link
Member

Choose a reason for hiding this comment

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

On purpose ?

@mfherbst
Copy link
Member

@nathanaelbosch I added a new todo to just add a sentence to the developer docs once this feature is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants