Add highlight component

[nkanderson]

Creates and renders a highlight component for the srcView component. A couple of things to note:

• I believe this could work for the SearchResults use of highlight with some additions, but I haven't attempted that yet
• One thing I feel a little uncertain about is the pulling of positional data from the DOM, in make_highlight (L65). It seems to be working fine, but I think if we're seeing the highlight end up in the wrong location, there may be some render lifecycle conditions where the final position isn't yet available. If that's the case, I think we'd need to move most of the make_highlight code into componentDidUpdate in srcView, and put positional data in state, pass down as props, etc.

[nrc]

This is a massive improvement, thanks! I think it would be good to remove any mutli-line highlight support, since that really contributes to the complexity. By getting rid, it would mean we only have highlight logic in the Highlight component, and that logic could be simpler, that might in turn make further refactoring clear.

[nkanderson]

Cool, yeah, I can do that. A couple questions:

• This means we can remove the "last line" calculation from the Highlight component, correct?
• The highlight logic for the Line component could be removed, but I think it would need to stay for LineNumber, right? I could look at refactoring it a bit though to determine the actual line number that should be highlighted, and pass that down as a prop, rather than passing highlight_start and highlight_end.

[nrc]

This means we can remove the "last line" calculation from the Highlight component, correct?

yep

but I think it would need to stay for LineNumber, right?

Hmm, yeah. I think it would be nice to have a highlighted prop on line number and have the owning component set that from the highlight, rather than have the line number check highlight. That sounds like what your proposing with the refactoring, yes?

[nkanderson]

@nrc since the position is now calculated differently, I adjusted the conditional for the left padding subtraction to check for left >=0 rather than left > 0. Do you know if there would ever be a case where left is less than zero, or could I remove the else here?

[nrc]

We call it here: https://github.com/nrc/cargo-src/pull/237/files/ac2f86ab6bc765177f3e12af715b890c5de80476#diff-4018adfb8de39d7a30dd31dd44e3b4cdR22 and lhs is highlight.column - 1, so I think that should always be greater than 0 since the column should be 1-indexed, but we might want to check that it really is 1-indexed, not 0-indexed, and that seems like an easy mistake to make so it might be worth asserting that highlight.column >= 1 there.

[nkanderson]

I found a case when testing a full-line highlight where lhs ends up being 0, and without changing the left > 0 (as it is in utils.js) to left >= 0, the highlight calculation is off slightly. But it sounds like we could just adjust the validate_highlight function to ensure highlight.column_start is not <= 1 rather than 0, as it is currently?

[nkanderson]

@nrc Ok, I'm re-thinking my previous comment - I think the validate_highlight logic can be left as-is, but the conditional adjustment in make_highlight could be reduced to left -= paddingLeft, with no check for left >= 0 or any else clause. At least that's working for the test cases I've been running through.