Page MenuHomePhabricator

Differential history mangles bullet numbering in quoted email forward
Closed, ResolvedPublic

Description

You can see all this history in http://reviews.llvm.org/D6292, which I believe is totally public.

I made a code review comment in the web client which included the following text:

Please fix this. At the very least, either:
1. Accept that ...
2. Specify some way ...
If you're doing #2 — and I think you probably should — a

This review was then forwarded to somebody using email. That forward shows up in the web-client history view, but somehow the numbering got messed up:

Please fix this. At the very least, either:
2. Accept that ...
3. Specify some way ...
If you're doing #2 — and I think you probably should —

There's also some crazy indentation going on in the quote.

Event Timeline

rjmccall raised the priority of this task from to Needs Triage.
rjmccall updated the task description. (Show Details)
rjmccall added a project: Differential.
rjmccall added a subscriber: rjmccall.

I'm guessing that this is a side-effect from rPHU0db4b149c4e0. Strange though. :/

Ah, no - I was wrong. It's because of the ---- line from the quoted section (- indicates a list item too). Observe:

> --
> Hello
> 1. One
> 2. Two

Hello

  1. One
  2. Two

Versus:

> Hello
> 1. One
> 2. Two

Hello

  1. One
  2. Two

A partial "fix" for this is to allow space before the divider rule, which turns --- into this:


Then the line of "------" in the mail would render as a divider instead of an empty list item. This would also fix quoting dividers in general, since this:

> ---

..becomes this right now:


...which is clearly unexpected. I think this is worth pursuing, and making sure the divider rule runs before the list rule so it wins in these cases (if it doesn't already) to pick up the side effect "fix" is probably worthwhile.

We also should possibly decline to begin lists when the first item is empty. It's sort of nice right now that you can type - and get a bullet in the preview without needing to enter text yet (and, e.g., check that # or 1. or other list markers are doing what you expect them to do), but it produces very odd results in this case.

I'm not sure if the slightly nicer preview behavior is worth more or less than the unusual result here.

Another approach would be to use a different set of divider characters (i.e., not hyphens).

epriestley triaged this task as Wishlist priority.Nov 19 2014, 7:45 PM
epriestley edited projects, added Remarkup; removed Differential.

My biggest objection is actually to the renumbering — I numbered my bullets precisely so that I could refer to them by number later. So maybe there's a way to say, "hey, I'm about to combine two lists in a way that causes a numbered list to be renumbered" and just not do that?

It's hard for me to imagine a case where somebody would really want:

  1. a
  2. b
  3. c
  4. d
  5. e
  6. f

to turn into 1, 2, 3, 4, 5, 6.

...yes, see, like that. That seems broken. That is not what I wrote.

(To clarify, in case you can't see the original, I wrote that as two sets, separated by a blank line, one 1,2,3 and then another 1,2,3 again.)

rPHU31e0327 partially fixed some of this. Specifically, > --- is no longer interpreted as a list item:


epriestley claimed this task.

This is extremely old and it's not clear to me which bad cases remain -- I can't immediately find any by fiddling. If any do remain, they haven't turned up again in about two years. If you're still seeing issues, feel free to file a new task with a modern test case.