Re: [PATCH v2 2/2] Add Rust support to source highlighting

Tom Tromey
2019-09-17T18:24:37Z

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Tested on openSUSE Leap 15.1, both with and without source-highlight
Tom> package installed.

Tom> OK for 8.3 branch?

This looks fine to me, but I had two comments.

First, Joel pointed out earlier that backports require a tracking PR, so
be sure to file one and mention it in the commit.

Second, up-thread you said:

Tom> This expection happens when the library attempts to access
Tom> /usr/share/source-highlight/esc.outlang, which is not there, because
Tom> it's contained in another package (source-highlight).

... but the patch does:

Tom> srchilite::SourceHighlight highlighter ("esc.outlang");
Tom> highlighter.setStyleFile("esc.style");

Tom> - std::ostringstream output;
Tom> - highlighter.highlight (input, output, lang_name, fullname);
Tom> + try
Tom> + {
Tom> + std::ostringstream output;
Tom> + highlighter.highlight (input, output, lang_name, fullname);

I am wondering if the "try" should encompass the construction of
"highlighter". It's possible that it works fine as-is, since maybe the
argument isn't used until highlighting is attempted -- but I figured I
would ask just to be sure.

thanks,
Tom