On Mon, Nov 25, 2024 at 03:02:21PM +0000, Carlos Maniero wrote:
> At this point HIGHLIGHT_SYNTAX is defaulting to source-highlight which
> will no longer be the expected result once we start executing unknown
> highlighters as commands.
>
> Signed-off-by: Carlos Maniero <car...@maniero.me>
> ---
> I see the docs were already updated, I don't know when the next release
> is scheduled. But what do you think to also add an warning if people are
> using HIGHLIGHT_SYNTAX with an unknown program?
Good idea.
> tp/ext/highlight_syntax.pm | 3 +++
> tp/tests/other/list-of-tests | 4 ++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
For the Perl part, three comments.
First, I think that this should be warning, not an error.
Second, on style, we try hard to be at 80 columns. It is not really
possible in that case, since the string is longer, but it is possible
to do better in that respect, for instance like:
if (defined($highlight_type) and $highlight_type ne 'source-highlight') {
$self->converter_document_error(sprintf(__(
'HIGHLIGHT_SYNTAX will no longer default to "source-highlight". Replace
"HIGHLIGHT_SYNTAX=%s" to "HIGHLIGHT_SYNTAX=source-highlight"'),
$highlight_type));
Third, on the message, in general, we try to use short messages, and if
it is not possible, at least split on different lines using the
continuation optional argument. In that case, for example, it seems to
me that the explanation is not needed, and it is possible to be shorter,
like:
"`%s' is not valid for HIGHLIGHT_SYNTAX. Set to `source-highlight'?"
>
> diff --git a/tp/ext/highlight_syntax.pm b/tp/ext/highlight_syntax.pm
> index 953f5c10eb..f22f9c610f 100644
> --- a/tp/ext/highlight_syntax.pm
> +++ b/tp/ext/highlight_syntax.pm
> @@ -80,6 +80,9 @@ sub highlight_setup($$)
> } elsif (defined($highlight_type) and $highlight_type eq 'pygments') {
> $cmd = 'pygmentize -L lexers';
> } else {
> + if (defined($highlight_type) and $highlight_type ne 'source-highlight') {
> + $self->converter_document_error(sprintf(__('HIGHLIGHT_SYNTAX will no
> longer default to "source-highlight". Replace "HIGHLIGHT_SYNTAX=%s" to
> "HIGHLIGHT_SYNTAX=source-highlight"'), $highlight_type));
> + }
> $highlight_type = 'source-highlight';
> $cmd = 'source-highlight --lang-list';
> }
> diff --git a/tp/tests/other/list-of-tests b/tp/tests/other/list-of-tests
> index 963befe46b..1a79af0e05 100644
> --- a/tp/tests/other/list-of-tests
> +++ b/tp/tests/other/list-of-tests
> @@ -1,10 +1,10 @@
>
> # syntax highlighting in examples
> -highlight_syntax_example highlight_example.texi --html -c HIGHLIGHT_SYNTAX=1
> +highlight_syntax_example highlight_example.texi --html -c
> HIGHLIGHT_SYNTAX=source-highlight
> highlight_syntax_example_pygments highlight_example.texi --html -c
> HIGHLIGHT_SYNTAX=pygments
> highlight_syntax_example_highlight highlight_example.texi --html -c
> HIGHLIGHT_SYNTAX=highlight
>
> -highlight_syntax_example_default_language highlight_example.texi --html -c
> HIGHLIGHT_SYNTAX=1 -c HIGHLIGHT_SYNTAX_DEFAULT_LANGUAGE=Perl
> +highlight_syntax_example_default_language highlight_example.texi --html -c
> HIGHLIGHT_SYNTAX=source-highlight -c HIGHLIGHT_SYNTAX_DEFAULT_LANGUAGE=Perl
>
> highlight_syntax_example_latin9 highlight_example.texi --html
> --init=highlight_syntax.pm -c 'OUTPUT_ENCODING_NAME=ISO-8859-15'
>
>
> base-commit: 49a0d9aa966f2515b6b783db6e9815f044abe849
> --
> 2.46.1
>
>
