[PATCH] diff view: improve appearance of line numbers

[Mads Kiilerich]

On 05/05/2015 06:15 PM, Andrew Shadura wrote:
> # HG changeset patch
> # User Andrew Shadura <andrew at shadura.me>
> # Date 1430842380 -7200
> #      Tue May 05 18:13:00 2015 +0200
> # Node ID c49fd027ae6491270795db939dcd724d6e29b3fd
> # Parent  4347c8958b9442dac9744e3f4e1e2282ffa25ce7
> diff view: improve appearance of line numbers
>
>   * display the ellipsis just once and centered

"instead of showing ellipsis in both the - and the + column" (I had to 
test to figure out what this meant)

>   * expand column width as needed to accomodate longer numbers

Does that fix an issue? In which case? Related to the other changes?

>   * enable box-sizing: border-box for the number links to
>     make it easier to set paddings

I have no idea about this. Is it related to the other changes? Which 
paddings were hard in which cases? How is it easier than before? Is it a 
trick we also could/should use elsewhere?

> diff --git a/kallithea/lib/diffs.py b/kallithea/lib/diffs.py
> --- a/kallithea/lib/diffs.py
> +++ b/kallithea/lib/diffs.py
> @@ -642,6 +642,7 @@ class DiffProcessor(object):
>   
>       def as_html(self, table_class='code-difftable', line_class='line',
>                   old_lineno_class='lineno old', new_lineno_class='lineno new',
> +                no_lineno_class='lineno',
>                   code_class='code', enable_comments=False, parsed_lines=None):
>           """
>           Return given diff as html table with customized css classes
> @@ -693,6 +694,8 @@ class DiffProcessor(object):
>                                   change['old_lineno'])
>                       cond_new = (change['new_lineno'] != '...' and
>                                   change['new_lineno'])
> +                    no_lineno = (change['old_lineno'] == '...' and
> +                                 change['new_lineno'] == '...')
>                       if cond_old:
>                           anchor_old_id = 'id="%s"' % anchor_old
>                       if cond_new:
> @@ -700,9 +703,10 @@ class DiffProcessor(object):
>                       ###########################################################
>                       # OLD LINE NUMBER
>                       ###########################################################
> -                    _html.append('''\t<td %(a_id)s class="%(olc)s">''' % {
> +                    _html.append('''\t<td %(a_id)s class="%(olc)s" %(colspan)s>''' % {
>                           'a_id': anchor_old_id,
> -                        'olc': old_lineno_class
> +                        'olc': no_lineno_class if no_lineno else old_lineno_class,
> +                        'colspan': 'colspan="2"' if no_lineno else ''
>                       })
>   
>                       _html.append('''%(link)s''' % {
> @@ -714,16 +718,17 @@ class DiffProcessor(object):
>                       # NEW LINE NUMBER
>                       ###########################################################
>   
> -                    _html.append('''\t<td %(a_id)s class="%(nlc)s">''' % {
> -                        'a_id': anchor_new_id,
> -                        'nlc': new_lineno_class
> -                    })
> +                    if not no_lineno:
> +                        _html.append('''\t<td %(a_id)s class="%(nlc)s">''' % {
> +                            'a_id': anchor_new_id,
> +                            'nlc': new_lineno_class
> +                        })
>   
> -                    _html.append('''%(link)s''' % {
> -                        'link': _link_to_if(True, change['new_lineno'],
> -                                            '#%s' % anchor_new)
> -                    })
> -                    _html.append('''</td>\n''')
> +                        _html.append('''%(link)s''' % {
> +                            'link': _link_to_if(True, change['new_lineno'],
> +                                                '#%s' % anchor_new)
> +                        })
> +                        _html.append('''</td>\n''')

It seems like it would be cleaner to have separate cases for with vs 
without line number.


Slightly related thought:
we need a way to link to and comment on binary files such - such as on 
https://kallithea-scm.org/repos/kallithea/changeset/4857d8f170d937383b8bde700494320fff80a88f 
.
I guess these not-even-ellipses lines also need some treatment ... or 
perhaps just that the first one should have line number 0.

/Mads

>                       ###########################################################
>                       # CODE
>                       ###########################################################
> diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
> --- a/kallithea/public/css/style.css
> +++ b/kallithea/public/css/style.css
> @@ -4906,7 +4906,6 @@ table.code-difftable td.code pre i {
>   table.code-difftable .lineno {
>       padding-left: 2px;
>       padding-right: 2px !important;
> -    text-align: right;
>       width: 30px;
>       -moz-user-select: none;
>       -webkit-user-select: none;
> @@ -4915,20 +4914,24 @@ table.code-difftable .lineno {
>       border-top: 0px solid #CCC !important;
>       border-bottom: none !important;
>       vertical-align: middle !important;
> +    text-align: center;
>   }
>   table.code-difftable .lineno.new {
> +    text-align: right;
>   }
>   table.code-difftable .lineno.old {
> +    text-align: right;
>   }
>   table.code-difftable .lineno a {
>       color: #aaa !important;
>       font: 11px Consolas, Monaco, Inconsolata, Liberation Mono, monospace !important;
>       letter-spacing: -1px;
> -    text-align: right;
> +    padding-left: 10px;
>       padding-right: 8px;
> +    box-sizing: border-box;
>       cursor: pointer;
>       display: block;
> -    width: 30px;
> +    width: 100%;
>   }
>   
>   table.code-difftable .lineno-inline {
> _______________________________________________
> kallithea-general mailing list
> kallithea-general at sfconservancy.org
> http://lists.sfconservancy.org/mailman/listinfo/kallithea-general