git-scm.com icon indicating copy to clipboard operation
git-scm.com copied to clipboard

Section link to 'ours' merge strategy is broken

Open DEVoytas opened this issue 8 years ago • 6 comments

There are two 'ours' section on https://git-scm.com/docs/merge-strategies page:

  • 'ours' as an option for 'recursive' merge strategy, and
  • 'ours' as separate merge strategy

One can grab link to any section by clicking on section sign icon that appears on a left-side of section header when mouse pointer is pointing there. However because both 'ours' section link to: https://git-scm.com/docs/merge-strategies#merge-strategies-ours it is impossible to grab a link to the actual 'ours' merge strategy. We always land on 'ours' as an option for 'recursive' merge as this is the first 'ours' section on the page.

DEVoytas avatar Mar 16 '17 11:03 DEVoytas

@peff this looks like an issue that needs to be resolved upstream in merge-strategies.txt so that asciidoc can generate better links, if I'm not mistaken.

sxlijin avatar Mar 16 '17 19:03 sxlijin

I don't think this is an upstream problem; the list anchors are a local git-scm.com thing (#849). The root of the problem is that the list anchors don't contain enough detail. There are two separate lists that both use the term ours, but the anchors don't reflect the full hierarchy of lists.

I'm not sure of the best way to do so. The solutions I can think of are:

  1. Name the anchor from the full root of the page, including intermediate items. That would make the first instance here something like #merge-strategies-recursive-ours. That covers this case, but it's certainly not foolproof. You might have two lists side-by-side. You could give each list its own name, but now things are starting to get ugly and complicated. And of course the names themselves can contain dashes, which screws up the hierarchy.

  2. Keep a hash of all of the anchors we've used for a given page, and number the duplicates. So the second one becomes merge-strategies-ours-2 or something. That's simple, but it's ugly (and it may change from version to version if we introduce new instances).

I dunno. I think (1) looks nicer, but it's not clear to me if we'd need to end up at (2) anyway. But maybe it would be a stopgap, and we can cross the other bridge when we come to it.

@buckelij, any thoughts as the original author?

peff avatar Mar 17 '17 23:03 peff

I agree option (1) is a lot nicer. I'm not too concerned about edge cases that break option (1): I think it would be "good enough".

Option (1) is a lot harder to implement the way we're doing it now. It really needs to be done in asciidoctor, which has knowledge of how things are nested.

I guess my preference is to not fix it until it can be done by asciidoctor itself. That will minimize the number of times we have to change/break the anchor links.

buckelij avatar Mar 20 '17 16:03 buckelij

Yeah, I was thinking that (1) might not be that hard. But we really don't have a true hierarchical view of the lists. It's all just regex hackery. I suppose it might be possible to feed it into an HTML parser, but I'd rather not go there if we can avoid it.

Looks like there hasn't been a lot of movement on asciidoctor/asciidoctor#1918, but it at least hasn't been shot down. So let's be patient for a while and see if anything happens there.

peff avatar Mar 20 '17 17:03 peff

Also, the ours and theirs options in the recursive strategy are not sufficiently indented. 'ours' is listed under 'recursive' which is listed under 'Merge Strategies,' but 'ours' is almost in line with 'recursive'.

image

talkbox6 avatar Apr 09 '17 23:04 talkbox6

@Solder-Soldier There's a separate issue for that: https://github.com/git/git-scm.com/issues/701

I think it's waiting on somebody to actually open a PR.

peff avatar Apr 10 '17 18:04 peff