On 06/20/2018 03:59 AM, John Keeping wrote:
On Tue, Jun 19, 2018 at 03:11:52AM +0800, Andy Green wrote:
On June 19, 2018 2:57:47 AM GMT+08:00, John Keeping <j...@keeping.me.uk> wrote:
On Mon, Jun 18, 2018 at 02:02:54PM +0800, Andy Green wrote:
Normal operation of blame view requires div.highlight to
have absolute position and set to its parent's top left
for me.

Otherwise the grey background boxes indicating the extent of
the patch in the lines td displace the highlit sources, they
start at the bottom of the td.

This patch makes the blame highlight div start back up the top of
its parent area and render on top of the grey boxes.

Checked on Linux Firefox 60 and Linux Chrome 69.

Which browser is this broken in?  I tried Linux Firefox 60 and
Chromium 67 and it looks ok without this patch.  (I'm not opposed to
the patch in principle, indeed it seems like a sensible change, but
I'm curious why I can't reproduce the problem.)

It's not caused in the browser, I checked the cgit site blame and that
doesn't have the problem.  I also went back to check just master
without anything on top, and it's still broken on my box.  I confirmed
I'm using the latest css, and that snipping the div with the boxes in
the lines td (using developer tools / inspector in ffox) is also
enough to have it display correctly.

Md2html seems to issue inline css style related to "highlight" as part
of its processing, I think as it is, that may put the actual rendering
behaviour at the mercy of pygments version or whatever actually
generates that.  So separating the css to a different name is likely
needed for robustness anyway.

Sounds sensible.  It looks like we use the same get_style_defs() call in
both syntax-hightlighing.py and md2html.  In syntax-hightlighing.py
the class name is generated by pygments [1] but in md2html we actually
specify the class as a parameter to the codehilite extension.

Your patch series allows these filters to run in the same page for the
first time, so I think what we should do is rename the highlight class
in md2html.  Does that also fix the problem?

Yes, it also resolves the naming conflict and it can display correctly.

I'll switch to this method then.

-Andy

[1] It may be possible to override it, I haven't checked the API docs

My box is fedora 28.

python3-pygments-2.2.0-10.fc28.noarch

"highlight" div class name is also used in md2html rendering
output.  So this patch solves it by introducing a wrapper
div and new "blame_highlight" css class.

Signed-off-by: Andy Green <a...@warmcat.com>
---
  cgit.css   |    2 ++
  ui-blame.c |    4 ++--
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/cgit.css b/cgit.css
index da8d9b0..5a85ceb 100644
--- a/cgit.css
+++ b/cgit.css
@@ -162,6 +162,8 @@ div#cgit table.list
tr.nohover-highlight:hover:nth-child(odd) {
        background: white;
  }
+div#cgit div.blame_highlight { position: absolute; top: 0; left: 0;
}

Is the "left" needed here?  I don't see any problem with setting the
position and top attributes, but setting left:0 moves the content right
up against the cell boundary where before we had a slight gap.

OK... it was not offset horizontally anyway so this is overkill.  I'll remove 
it.

-Andy

+
  div#cgit table.list th {
        font-weight: bold;
        /* color: #888;
diff --git a/ui-blame.c b/ui-blame.c
index 6e23f0b..ab44e3f 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -196,7 +196,7 @@ static void print_object(const struct object_id
*oid, const char *path,
        free((void *)sb.final_buf);
/* Lines */
-       html("<pre><code>");
+       html("<div class=\"blame_highlight\"> <pre><code>");

No need for a space before <pre> here.

        if (ctx.repo->source_filter) {
                char *filter_arg = xstrdup(basename);
                cgit_open_filter(ctx.repo->source_filter, filter_arg);
@@ -207,7 +207,7 @@ static void print_object(const struct object_id
*oid, const char *path,
                html_txt(buf);
        }
- html("</code></pre>");
+       html("</code></pre></div>");
html("</div></td>\n");
_______________________________________________
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
_______________________________________________
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Reply via email to