Open mgeisler opened 6 months ago
- Make
mdbook-xgettext
optional withoptional: true
inbook.toml
. Make it exit early when it sees that it's optional. If this works well, we could use it for themdbook-pandoc
output too. See Pandoc failure during local rendering comprehensive-rust#1911 for a recent example of it failing.
@max-heller, what do you think of taking advantage of optional: true
like this?
I'm basically suggesting that "heavy" output formats like mdbook-pandoc
and mdbook-xgettext
could change the semantics of this setting to mean: I won't run if I'm optional.
People would then have to override it back to false
with a command like this:
MDBOOK_OUTPUT__FOO__OPTIONAL=false mdbook build
This would mean that mdbook build
works with both when mdbook-foo
is not installed or when the mdbook-foo
dependencies are not installed since mdbook-foo
will exit immediately. Only if you ask very clearly for mdbook-foo
to do something, will it attempt to execute.
We could even make this more clear by letting the output formats do this if another config value is set:
[output.foo]
optional = true
run-when-optional = false
It would all be convention, but it feels like it could be a useful convention for rarely used output formats like the ones discussed here?
I'm basically suggesting that "heavy" output formats like
mdbook-pandoc
andmdbook-xgettext
could change the semantics of this setting to mean: I won't run if I'm optional.People would then have to override it back to
false
with a command like this:MDBOOK_OUTPUT__FOO__OPTIONAL=false mdbook build
While these semantics (and an associated way to run a specific renderer) would make sense to me if mdbook
were to adopt them, I don't think it makes sense to override them to conflict with the standard semantics.
We could even make this more clear by letting the output formats do this if another config value is set:
[output.foo] optional = true run-when-optional = false
This makes more sense to me because it is opt-in and preserves the optional
semantics in the default case.
Some other thoughts:
optional
as "make a best-effort attempt" and exit "successfully" with a warning when they otherwise would've exited unsuccessfully. This preserves the happy case semantics but avoids breaking the build when the renderer can't complete successfully for some reason. Maybe this is something mdbook
itself could adopt backwards-compatibly?It seems like there are two cases we'd like to support:
mdbook-xgettext
from what I can tell). This is better served by "don't run at all when optional" semanticsmdbook-pandoc
). This may be better served by "don't break the build when optional" semanticsIt'd be nice to come up with a scheme that could accomplish both. I think interpreting optional
as "don't break the build" and using run-if-optional
to prevent running altogether might do the trick?
- It'd be nice to come up with a scheme that could accomplish both. I think interpreting
optional
as "don't break the build" and usingrun-if-optional
to prevent running altogether might do the trick?
Yeah, using two options together should work nicely. What I like is that I can hard code everything in the book.toml
file and get nice behavior out of the box for people who haven't installed all dependencies.
Bikeshed: how about skip-if-optional = true
instead of run-if-optional = false
?
Bikeshed: how about
skip-if-optional = true
instead ofrun-if-optional = false
?
I'm happy with skip-if-optional
!
Thinking about this some more, it feels odd to have optional
and skip-if-optional
(logically, A
and A => B
) instead of optional
and another option that disables the renderer (disabled
?) independently of optional
(logically, A
and B
)
Thinking about this some more, it feels odd to have
optional
andskip-if-optional
(logically,A
andA => B
) instead ofoptional
and another option that disables the renderer (disabled
?) independently ofoptional
(logically,A
andB
)
@mgeisler any opinions on this?
Thinking about this some more, it feels odd to have
optional
andskip-if-optional
(logically,A
andA => B
) instead ofoptional
and another option that disables the renderer (disabled
?) independently ofoptional
(logically,A
andB
)@mgeisler any opinions on this?
Sorry for the delay! Yeah, you're right: having a simple disabled
setting would make more sense! So hard-coding optional = true
and disabled = true
would be useful for the situation where one wants to
optional = true
makes mdbook
happy)disabled = true
handles this)I'll be happy to implement this for mdbook-xgettext
as well then.
For Comprehensive Rust, we use a
granularity: 0
setting when extracting messages withmdbook-xgettext
. This avoids churn in the line numbers.However, it happens regularly (https://github.com/google/comprehensive-rust/pull/2100, https://github.com/google/comprehensive-rust/pull/1991, https://github.com/google/comprehensive-rust/pull/1950, ...) that people forget to use this setting.
I would love to have a way of specifying the right settings in a configuration file. Something which can turn
into just
or similar.
We could already store the options in our
book.toml
file today. The reason we don't do this is that this would enable thexgettext
output unconditionally for everybody.Some ideas for solving this:
mdbook-xgettext
required and enabled in every execution. This would be a little wasteful, but it might not matter if it's quick enough? Measure this to get some data.mdbook-xgettext
optional withoptional: true
inbook.toml
. Make it exit early when it sees that it's optional. If this works well, we could use it for themdbook-pandoc
output too. See https://github.com/google/comprehensive-rust/issues/1911 for a recent example of it failing.mdbook-xgettext
directly using the above config hack.