Line context for GitHub links in onebox

Product Support

AI-generated summary

The discussion revolves around the Line context for GitHub links in onebox feature. guildai wants to show context for a range of lines in a GitHub link, similar to how it works for a single line. Currently, onebox shows context only for single lines. codinghorror agrees that the behavior is inconsistent, but sam points out that it’s not configurable at the moment.

guildai explains that they need to highlight multiple lines with context to step the reader through changes in a file. They propose adding configuration options, such as data-onebox-highlight and data-onebox-context, to control the behavior.

sam suggests a possible workaround by adding a query parameter to the URL, but guildai prefers an explicit method for controlling the behavior. They implemented a theme component as a workaround, but believe that an explicit method is better than changing defaults or making the component smarter.

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?

2

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

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).

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.

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.