Comment 3 for bug 306631

On 17:54 Sun 01 Mar , Michael Hudson wrote:
> That looks pretty good!
>
> Some comments;
>
> 1) Why do you have a class (PygmentsHighlighter) with a single
> staticmethod? This is Python, not Java, a free function would be fine
> :)

I think my orignal intention was that we might have different
highlighting modules in the future, so we might use
xHighlighter.highlight instead of PygmentsHighlighter.highlight and that
the class may have more than one method... but yeah, as it is, a free
function is probably fine.

> 2) Would you be prepared/able to assign the copyright to Canonical for
> this? Loggerhead's copyright situation is a bit murky, but we are
> trying to make it clearer, gradually...

This would be fine, as long as I get some credit somewhere (NEWS file or
whatever).

> 3) I wouldn't worry about themes at all for now. Just make it look OK
> with the default loggerhead look (which I think it does OK already in
> fact).

I guess the next step is to address 1) which is easy enough and to
perhaps make it work for the diff view. I did run into a slight problem
where the highlighting seems to eat up the whole CPU if it doesn't
quite know what the file is (for me this was a CUDA .cu file), but that
is a problem with pygments and not loggerhead. Perhaps it needs more
testing.

> I can tweak the code a bit, so I guess it's only really 2) I need an
> answer to.
>
> --
> Make code and diffs easier to read using syntax highlighting and other font options
> https://bugs.launchpad.net/bugs/306631
> You received this bug notification because you are a direct subscriber
> of the bug.

--
=======================================================================
 Peter Bui (<email address hidden>)
=======================================================================