Line context for GitHub links in onebox

support
1

If a single line is specified for a GitHub link, onebox shows that line with context:

https://github.com/guildai/guildai/blob/master/examples/get-started/mnist_mlp.py#L16

If multiple lines are specified, onebox shows those without context:

https://github.com/guildai/guildai/blob/master/examples/get-started/mnist_mlp.py#L16-L18

I understand the rationale for the different here. However, I’d like to show context for the lines range. Is there a weird trick for this?

Also, is there a way to configure the number of context lines shown?

5 Likes
2

I suppose it would depend how many lines you’re specifying (5? 500?), but I agree it is a little inconsistent.

3 Likes
3

This is not configurable at the moment. I wonder though, why bother with context if you can just ask for L12-L20 ?

This is a bit of a 2 feature requests in one :slight_smile: I guess you want it to “highlight” the rows and then add context around it? I guess this can be useful for 2-5 line selection, but a huge chunk in yellow would be pretty loud.

4

In this case I’m stepping the reader through a series of changes to a file. The context is important in each case, whether I’m highlighting one line or many.

The component already highlights one line with context. The case I have here would like that same behavior applied with multiple lines.

I see the sensibility of the current scheme but I think there’s a solid case to support multi-line highlights w/context as well.

If Onebox isn’t already configurable beyond the link itself, it would need to be. Perhaps something like this?

<span
  data-onebox-highlight="true"
  data-onebox-context="5"
  >https://github.com/.../get-started/mnist_mlp.py#L16-L18</span>

There’s not much new here other than configuring what the component formats and displays.

Naively, it looks like this would be a backend only change. Consider the differences between these two onebox.json responses:

As one might expect, one has context with a highlight line and the other does not.

If the backend were configurable per request, it’s a matter of applying the existing formatting rules. I see the implementation assumes one line-to-highlight. The change to support a line range is minimal (e.g. use an array of lines-to-highlight and check for membership).

1 Like
5

Maybe something like this will work for these cases, if they are 100% outliers, trouble is that it fights with GitHub, which is not great.

https://github.com/.../get-started/mnist_mlp.py?#L16-L18-context-3

Or even maybe just include context out of the box if the region is 4 lines or less.

1 Like
6

Personally I’d avoid implicit behavior. I already have an example that highlights five lines :slight_smile:

I implemented a workaround for this, which uses a theme component that looks for a wrapper div containing config for the wrapped GitHub link. The component waits on the DOM modified event to know when the link is replaced with code lines and then works on the li.selected state to select. This hack doesn’t change the lines retrieved so it can’t add/remove context.

I think perhaps an explicit method for controlling the behavior is better than changing any of the defaults or trying the make the component smarter.

Conveying the config/settings in the URL itself could work but it undermines the semantic meaning of the link. If using a wrapping tag isn’t an option, I assume you’d want to at least strip off onebox-specific config.