[PATCH] revpanel: add linkpattern config to customize the target of cset links (fixes #3373)

[Angel Ezquerra]

# HG changeset patch
# User Angel Ezquerra <angel.e...@gmail.com>
# Date 1381344134 -7200
# Wed Oct 09 20:42:14 2013 +0200
# Node ID b9a91bd64ee118cf83560f981790e6d18b6ab120
# Parent 0714d3ca79787421184c34883f31858505dfa31a
revpanel: add linkpattern config to customize the target of cset links (fixes #3373)

Issue #3373 suggests that it would be nice to be able to customize the changeset
link on the revpanel so that it takes you go a web page for example.

In order to do so, we introduce a tortoisehg/linkpattern config setting, which
is a "pattern string" that serves as a pattern to generate the actual link URL.

The linkpattern will be interpolated using the following operation:

link = linkpattern % {'revid': revid, 'revnum': revnum}

That is, the linkpattern can refer to the revid and/or the revnum using
"%(revid)s" and "%(revnum)d". For example:

linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s

can be used to link cset links to their corresponding bitbucket commit page.

Currently there is no GUI for this configuration option. The best settings panel
for this seems to be the Workbench panel, but it is quite crowded already.

diff --git a/tortoisehg/hgqt/revpanel.py b/tortoisehg/hgqt/revpanel.py
--- a/tortoisehg/hgqt/revpanel.py
+++ b/tortoisehg/hgqt/revpanel.py
@@ -102,14 +102,16 @@
raise csinfo.UnknownItem(item)

def markup_func(widget, item, value):
- def link_markup(revnum, revid, enable=True):
+ def link_markup(revnum, revid, enable=True, linkpattern=None):
mrevid = revid_markup('%s (%s)' % (revnum, revid))
if not enable:
return mrevid
- link = 'cset:%s' % revid
+ if not linkpattern:
+ linkpattern = 'cset:%(revid)s'
+ link = linkpattern % {'revid': revid, 'revnum': revnum}
return '<a href="%s">%s</a>' % (link, mrevid)
def revline_markup(revnum, revid, summary, highlight=None,
- branch=None, link=True):
+ branch=None, link=True, linkpattern=None):
def branch_markup(branch):
opts = dict(fg='black', bg='#aaffaa')
return qtlib.markup(' %s ' % branch, **opts)
@@ -117,7 +119,7 @@
if branch:
branch = branch_markup(branch)
if revid:
- rev = link_markup(revnum, revid, link)
+ rev = link_markup(revnum, revid, link, linkpattern=linkpattern)
if branch:
return '%s %s %s' % (rev, branch, summary)
return '%s %s' % (rev, summary)
@@ -128,10 +130,15 @@
return '%s - %s' % (revnum, summary)
if item in ('cset', 'graft', 'transplant', 'mqoriginalparent',
'p4', 'svn', 'converted'):
- link = item != 'cset'
+ if item == 'cset':
+ linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
+ link = linkpattern is not None
+ else:
+ linkpattern = None
+ link = True
if isinstance(value, basestring):
return revid_markup(value)
- return revline_markup(link=link, *value)
+ return revline_markup(link=link, linkpattern=linkpattern, *value)
elif item in ('parents', 'children', 'precursors', 'successors'):
csets = []
for cset in value:

[Yuya Nishihara]

On Wed, 09 Oct 2013 20:42:29 +0200, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.e...@gmail.com>
> # Date 1381344134 -7200
> # Wed Oct 09 20:42:14 2013 +0200
> # Node ID b9a91bd64ee118cf83560f981790e6d18b6ab120
> # Parent 0714d3ca79787421184c34883f31858505dfa31a
> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
>
> Issue #3373 suggests that it would be nice to be able to customize the changeset
> link on the revpanel so that it takes you go a web page for example.
>
> In order to do so, we introduce a tortoisehg/linkpattern config setting, which
> is a "pattern string" that serves as a pattern to generate the actual link URL.
>
> The linkpattern will be interpolated using the following operation:
>
> link = linkpattern % {'revid': revid, 'revnum': revnum}
>
> That is, the linkpattern can refer to the revid and/or the revnum using
> "%(revid)s" and "%(revnum)d". For example:
>
> linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s

We shouldn't introduce new template syntax. Mercurial has one.

http://selenic.com/repo/hg/help/templating

> + if item == 'cset':
> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
> + link = linkpattern is not None

It smells bad assuming that widget has repo attribute.

Regards,

[Angel Ezquerra]

On Thu, Oct 10, 2013 at 2:49 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> On Wed, 09 Oct 2013 20:42:29 +0200, Angel Ezquerra wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <angel.e...@gmail.com>
>> # Date 1381344134 -7200
>> # Wed Oct 09 20:42:14 2013 +0200
>> # Node ID b9a91bd64ee118cf83560f981790e6d18b6ab120
>> # Parent 0714d3ca79787421184c34883f31858505dfa31a
>> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
>>
>> Issue #3373 suggests that it would be nice to be able to customize the changeset
>> link on the revpanel so that it takes you go a web page for example.
>>
>> In order to do so, we introduce a tortoisehg/linkpattern config setting, which
>> is a "pattern string" that serves as a pattern to generate the actual link URL.
>>
>> The linkpattern will be interpolated using the following operation:
>>
>> link = linkpattern % {'revid': revid, 'revnum': revnum}
>>
>> That is, the linkpattern can refer to the revid and/or the revnum using
>> "%(revid)s" and "%(revnum)d". For example:
>>
>> linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s
>
> We shouldn't introduce new template syntax. Mercurial has one.

This is not a new template syntax. This is the regular python string
interpolation syntax, supported all the way back to Python 2.3 I
believe.

> http://selenic.com/repo/hg/help/templating
>
>> + if item == 'cset':
>> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
>> + link = linkpattern is not None
>
> It smells bad assuming that widget has repo attribute.

Would you think it would be better to pass repo as an explicit
parameter to markup_func?

Cheers,

Angel

[Yuya Nishihara]

On Thu, 10 Oct 2013 16:03:57 +0200, Angel Ezquerra wrote:
> On Thu, Oct 10, 2013 at 2:49 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> > On Wed, 09 Oct 2013 20:42:29 +0200, Angel Ezquerra wrote:
> >> # HG changeset patch
> >> # User Angel Ezquerra <angel.e...@gmail.com>
> >> # Date 1381344134 -7200
> >> # Wed Oct 09 20:42:14 2013 +0200
> >> # Node ID b9a91bd64ee118cf83560f981790e6d18b6ab120
> >> # Parent 0714d3ca79787421184c34883f31858505dfa31a
> >> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
> >>
> >> Issue #3373 suggests that it would be nice to be able to customize the changeset
> >> link on the revpanel so that it takes you go a web page for example.
> >>
> >> In order to do so, we introduce a tortoisehg/linkpattern config setting, which
> >> is a "pattern string" that serves as a pattern to generate the actual link URL.
> >>
> >> The linkpattern will be interpolated using the following operation:
> >>
> >> link = linkpattern % {'revid': revid, 'revnum': revnum}
> >>
> >> That is, the linkpattern can refer to the revid and/or the revnum using
> >> "%(revid)s" and "%(revnum)d". For example:
> >>
> >> linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s
> >
> > We shouldn't introduce new template syntax. Mercurial has one.
>
> This is not a new template syntax. This is the regular python string
> interpolation syntax, supported all the way back to Python 2.3 I
> believe.

It's new to config file.
Also printf-style formatting can easily raise exception.

> > http://selenic.com/repo/hg/help/templating
> >
> >> + if item == 'cset':
> >> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
> >> + link = linkpattern is not None
> >
> > It smells bad assuming that widget has repo attribute.
>
> Would you think it would be better to pass repo as an explicit
> parameter to markup_func?

or bind repo or ui to markup_func?
It looks widget.repo is kept only for csinfo.SummaryBase.update().

Regards,

[Steve Borho]

On Wed, Oct 9, 2013 at 1:42 PM, Angel Ezquerra <angel.e...@gmail.com> wrote:

# HG changeset patch
# User Angel Ezquerra <angel.e...@gmail.com>
# Date 1381344134 -7200
#      Wed Oct 09 20:42:14 2013 +0200
# Node ID b9a91bd64ee118cf83560f981790e6d18b6ab120
# Parent  0714d3ca79787421184c34883f31858505dfa31a
revpanel: add linkpattern config to customize the target of cset links (fixes #3373)

Issue #3373 suggests that it would be nice to be able to customize the changeset
link on the revpanel so that it takes you go a web page for example.

In order to do so, we introduce a tortoisehg/linkpattern config setting, which
is a "pattern string" that serves as a pattern to generate the actual link URL.

The linkpattern will be interpolated using the following operation:

    link = linkpattern % {'revid': revid, 'revnum': revnum}

That is, the linkpattern can refer to the revid and/or the revnum using
"%(revid)s" and "%(revnum)d". For example:

    linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s

can be used to link cset links to their corresponding bitbucket commit page.

Currently there is no GUI for this configuration option. The best settings panel
for this seems to be the Workbench panel, but it is quite crowded already.

Speaking of which; all of these Workbench items could be moved to a new Sync tab

* After Pull Operation

* Default PUsh

* Confirm Push

* Target combo (needs caps)

 

--
You received this message because you are subscribed to the Google Groups "TortoiseHg Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to thg-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Steve Borho

[Angel Ezquerra]

Steve,

I'm a bit confused. Isn't there a Sync tab containing those settings already?

Cheers,

Angel

[Angel Ezquerra]

What do you propose then? Perhaps we should just replace "{node}" with
"revid" and "{rev}" with "revnum"?

>> > http://selenic.com/repo/hg/help/templating
>> >
>> >> + if item == 'cset':
>> >> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
>> >> + link = linkpattern is not None
>> >
>> > It smells bad assuming that widget has repo attribute.
>>
>> Would you think it would be better to pass repo as an explicit
>> parameter to markup_func?
>
> or bind repo or ui to markup_func?
> It looks widget.repo is kept only for csinfo.SummaryBase.update().

Looking again at the code, it is not that clear to me why you think we
cannot assume that the widget will have a repo in revpanel.py. It is
clearly passed from the csinfo.create function down to the widget
creation...

On the other hand, it is not very explicit how or when the markup_func
is actually called. Changing its interface seems riskier to me,
specially since there are other places where a markup function is
defined...

Angel

[Yuya Nishihara]

On Sat, 12 Oct 2013 00:26:50 +0200, Angel Ezquerra wrote:
> On Thu, Oct 10, 2013 at 6:06 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> > On Thu, 10 Oct 2013 16:03:57 +0200, Angel Ezquerra wrote:
> >> On Thu, Oct 10, 2013 at 2:49 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> >> >> That is, the linkpattern can refer to the revid and/or the revnum using
> >> >> "%(revid)s" and "%(revnum)d". For example:
> >> >>
> >> >> linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s
> >> >
> >> > We shouldn't introduce new template syntax. Mercurial has one.
> >>
> >> This is not a new template syntax. This is the regular python string
> >> interpolation syntax, supported all the way back to Python 2.3 I
> >> believe.
> >
> > It's new to config file.
> > Also printf-style formatting can easily raise exception.
>
> What do you propose then? Perhaps we should just replace "{node}" with
> "revid" and "{rev}" with "revnum"?

I prefer it because

- we can port Mercurial's template system later
- customtools have similar syntax, though variable names are different

> >> > http://selenic.com/repo/hg/help/templating
> >> >
> >> >> + if item == 'cset':
> >> >> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
> >> >> + link = linkpattern is not None
> >> >
> >> > It smells bad assuming that widget has repo attribute.
> >>
> >> Would you think it would be better to pass repo as an explicit
> >> parameter to markup_func?
> >
> > or bind repo or ui to markup_func?
> > It looks widget.repo is kept only for csinfo.SummaryBase.update().
>
> Looking again at the code, it is not that clear to me why you think we
> cannot assume that the widget will have a repo in revpanel.py. It is
> clearly passed from the csinfo.create function down to the widget
> creation...

For example, SummaryBase has ctx attribute, but data_func receives ctx as
an argument. This implies accessing widget.ctx violates the original design
of csinfo. I don't see widget.repo has different scope than widget.ctx.

> On the other hand, it is not very explicit how or when the markup_func
> is actually called. Changing its interface seems riskier to me,
> specially since there are other places where a markup function is
> defined...

So I suggest to bind "ui" to markup_func, which might be dirty hack,
but can avoid vast redesign.

-def markup_func(widget, item, value):
+def create_markup_func(ui):
def link_markup(revnum, revid, enable=True):

mrevid = revid_markup('%s (%s)' % (revnum, revid))

...
+
+ def markup_func(widget, item, value):
+ if item in ('cset', 'graft', 'transplant', 'mqoriginalparent',
...
+ return markup_func
...
def RevPanelWidget(repo):
'''creates a rev panel widget and returns it'''
custom = csinfo.custom(data=data_func, label=label_func,
- markup=markup_func)
+ markup=create_markup_func(repo.ui))

or

class markup_func(object):
def __init__(self, ui):
self.ui = ui
...
def __call__(self, widget, item, value):
...

Regards,

[Angel Ezquerra]

# HG changeset patch
# User Angel Ezquerra <angel.e...@gmail.com>
# Date 1381344134 -7200
# Wed Oct 09 20:42:14 2013 +0200

# Node ID 7bf3715d98a4564f4fe2299096660ebdc30e84e4
# Parent a9b7bcd86ac1937ffd825c9fb63ecf89f994e266

revpanel: add linkpattern config to customize the target of cset links (fixes #3373)

Issue #3373 suggests that it would be nice to be able to customize the changeset
link on the revpanel so that it takes you go a web page for example.

In order to do so, we introduce a tortoisehg/linkpattern config setting, which
is a "pattern string" that serves as a pattern to generate the actual link URL.

The linkpattern uses a mercurial "template like" syntax that currently accepts
two template variables: {node}, which is replaced by the 40 digit revision id,
and {rev} which is the revision number.

For example:

linkpattern = https://bitbucket.org/tortoisehg/thg/commits/{node}

can be used to link cset links to their corresponding bitbucket commit page.

Currently there is no GUI for this configuration option. The best settings panel
for this seems to be the Workbench panel, but it is quite crowded already.

diff --git a/tortoisehg/hgqt/revpanel.py b/tortoisehg/hgqt/revpanel.py
--- a/tortoisehg/hgqt/revpanel.py
+++ b/tortoisehg/hgqt/revpanel.py

@@ -101,15 +101,17 @@

raise csinfo.UnknownItem(item)

-def markup_func(widget, item, value):

- def link_markup(revnum, revid, enable=True):

+def create_markup_func(ui):

+ def link_markup(revnum, revid, enable=True, linkpattern=None):

mrevid = revid_markup('%s (%s)' % (revnum, revid))

if not enable:
return mrevid
- link = 'cset:%s' % revid
+ if not linkpattern:

+ linkpattern = 'cset:{node}'
+ link = linkpattern.replace('{node}', revid).replace('{revnum}', revnum)

return '<a href="%s">%s</a>' % (link, mrevid)
def revline_markup(revnum, revid, summary, highlight=None,
- branch=None, link=True):
+ branch=None, link=True, linkpattern=None):
def branch_markup(branch):
opts = dict(fg='black', bg='#aaffaa')
return qtlib.markup(' %s ' % branch, **opts)
@@ -117,7 +119,7 @@
if branch:
branch = branch_markup(branch)
if revid:
- rev = link_markup(revnum, revid, link)
+ rev = link_markup(revnum, revid, link, linkpattern=linkpattern)
if branch:
return '%s %s %s' % (rev, branch, summary)
return '%s %s' % (rev, summary)

@@ -126,26 +128,33 @@
if branch:
return '%s - %s %s' % (revnum, branch, summary)

return '%s - %s' % (revnum, summary)

- if item in ('cset', 'graft', 'transplant', 'mqoriginalparent',
- 'p4', 'svn', 'converted'):

- link = item != 'cset'

- if isinstance(value, basestring):
- return revid_markup(value)
- return revline_markup(link=link, *value)
- elif item in ('parents', 'children', 'precursors', 'successors'):
- csets = []
- for cset in value:
- if isinstance(cset, basestring):
- csets.append(revid_markup(cset))

+ def markup_func(widget, item, value):
+ if item in ('cset', 'graft', 'transplant', 'mqoriginalparent',

+ 'p4', 'svn', 'converted'):

+ if item == 'cset':

+ linkpattern = ui.config('tortoisehg', 'linkpattern', None)

+ link = linkpattern is not None

else:
- csets.append(revline_markup(*cset))
- return csets
- raise csinfo.UnknownItem(item)

+ linkpattern = None
+ link = True

+ if isinstance(value, basestring):
+ return revid_markup(value)

+ return revline_markup(link=link, linkpattern=linkpattern, *value)

+ elif item in ('parents', 'children', 'precursors', 'successors'):
+ csets = []
+ for cset in value:
+ if isinstance(cset, basestring):
+ csets.append(revid_markup(cset))
+ else:
+ csets.append(revline_markup(*cset))
+ return csets
+ raise csinfo.UnknownItem(item)
+ return markup_func

def RevPanelWidget(repo):
'''creates a rev panel widget and returns it'''
custom = csinfo.custom(data=data_func, label=label_func,
- markup=markup_func)
+ markup=create_markup_func(repo.ui))

style = csinfo.panelstyle(contents=('cset', 'branch', 'obsolete', 'close', 'user',
'dateage', 'parents', 'children', 'tags', 'graft', 'transplant',
'mqoriginalparent',

[Angel Ezquerra]

On Sat, Oct 12, 2013 at 7:33 AM, Yuya Nishihara <yu...@tcha.org> wrote:
> On Sat, 12 Oct 2013 00:26:50 +0200, Angel Ezquerra wrote:
>> On Thu, Oct 10, 2013 at 6:06 PM, Yuya Nishihara <yu...@tcha.org> wrote:
>> > On Thu, 10 Oct 2013 16:03:57 +0200, Angel Ezquerra wrote:
>> >> On Thu, Oct 10, 2013 at 2:49 PM, Yuya Nishihara <yu...@tcha.org> wrote:
>> >> >> That is, the linkpattern can refer to the revid and/or the revnum using
>> >> >> "%(revid)s" and "%(revnum)d". For example:
>> >> >>
>> >> >> linkpattern = https://bitbucket.org/tortoisehg/thg/commits/%(revid)s
>> >> >
>> >> > We shouldn't introduce new template syntax. Mercurial has one.
>> >>
>> >> This is not a new template syntax. This is the regular python string
>> >> interpolation syntax, supported all the way back to Python 2.3 I
>> >> believe.
>> >
>> > It's new to config file.
>> > Also printf-style formatting can easily raise exception.
>>
>> What do you propose then? Perhaps we should just replace "{node}" with
>> "revid" and "{rev}" with "revnum"?
>
> I prefer it because
>
> - we can port Mercurial's template system later
> - customtools have similar syntax, though variable names are different

OK.

>> >> > http://selenic.com/repo/hg/help/templating
>> >> >
>> >> >> + if item == 'cset':
>> >> >> + linkpattern = widget.repo.ui.config('tortoisehg', 'linkpattern', None)
>> >> >> + link = linkpattern is not None
>> >> >
>> >> > It smells bad assuming that widget has repo attribute.
>> >>
>> >> Would you think it would be better to pass repo as an explicit
>> >> parameter to markup_func?
>> >
>> > or bind repo or ui to markup_func?
>> > It looks widget.repo is kept only for csinfo.SummaryBase.update().
>>
>> Looking again at the code, it is not that clear to me why you think we
>> cannot assume that the widget will have a repo in revpanel.py. It is
>> clearly passed from the csinfo.create function down to the widget
>> creation...
>
> For example, SummaryBase has ctx attribute, but data_func receives ctx as
> an argument. This implies accessing widget.ctx violates the original design
> of csinfo. I don't see widget.repo has different scope than widget.ctx.

OK, that is pretty convincing :-)

>> On the other hand, it is not very explicit how or when the markup_func
>> is actually called. Changing its interface seems riskier to me,
>> specially since there are other places where a markup function is
>> defined...
>
> So I suggest to bind "ui" to markup_func, which might be dirty hack,
> but can avoid vast redesign.
>
> -def markup_func(widget, item, value):
> +def create_markup_func(ui):
> def link_markup(revnum, revid, enable=True):
> mrevid = revid_markup('%s (%s)' % (revnum, revid))
> ...
> +
> + def markup_func(widget, item, value):
> + if item in ('cset', 'graft', 'transplant', 'mqoriginalparent',
> ...
> + return markup_func
> ...
> def RevPanelWidget(repo):
> '''creates a rev panel widget and returns it'''
> custom = csinfo.custom(data=data_func, label=label_func,
> - markup=markup_func)
> + markup=create_markup_func(repo.ui))
>

That is very neat! Thanks!

Angel

[Yuya Nishihara]

On Sun, 13 Oct 2013 01:25:17 +0200, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.e...@gmail.com>
> # Date 1381344134 -7200
> # Wed Oct 09 20:42:14 2013 +0200
> # Node ID 7bf3715d98a4564f4fe2299096660ebdc30e84e4
> # Parent a9b7bcd86ac1937ffd825c9fb63ecf89f994e266
> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
>
> Issue #3373 suggests that it would be nice to be able to customize the changeset
> link on the revpanel so that it takes you go a web page for example.
>
> In order to do so, we introduce a tortoisehg/linkpattern config setting, which
> is a "pattern string" that serves as a pattern to generate the actual link URL.
>
> The linkpattern uses a mercurial "template like" syntax that currently accepts
> two template variables: {node}, which is replaced by the 40 digit revision id,
> and {rev} which is the revision number.

Actually {node} is replaced by the short hash. ;)

> +def create_markup_func(ui):
> + def link_markup(revnum, revid, enable=True, linkpattern=None):
> mrevid = revid_markup('%s (%s)' % (revnum, revid))
> if not enable:
> return mrevid
> - link = 'cset:%s' % revid
> + if not linkpattern:
> + linkpattern = 'cset:{node}'
> + link = linkpattern.replace('{node}', revid).replace('{revnum}', revnum)

It should be '{rev}'.

> + if item == 'cset':
> + linkpattern = ui.config('tortoisehg', 'linkpattern', None)
> + link = linkpattern is not None
> else:
> - csets.append(revline_markup(*cset))
> - return csets
> - raise csinfo.UnknownItem(item)
> + linkpattern = None
> + link = True

Probably you can merge "link" (boolean) and "linkpattern" like this:

if item == 'cset':
linkpattern = ui.config('tortoisehg', 'linkpattern')
else:
linkpattern = 'cset:{node}'

FWIW, it needs more specific name. How about 'tortoisehg.changesetlink'
or 'tortoisehg.changeset.link'? There's already 'tortoisehg.issue.link'
setting.

> + if isinstance(value, basestring):
> + return revid_markup(value)
> + return revline_markup(link=link, linkpattern=linkpattern, *value)
> + elif item in ('parents', 'children', 'precursors', 'successors'):
> + csets = []
> + for cset in value:
> + if isinstance(cset, basestring):
> + csets.append(revid_markup(cset))
> + else:
> + csets.append(revline_markup(*cset))
> + return csets
> + raise csinfo.UnknownItem(item)
> + return markup_func
>
> def RevPanelWidget(repo):
> '''creates a rev panel widget and returns it'''
> custom = csinfo.custom(data=data_func, label=label_func,
> - markup=markup_func)
> + markup=create_markup_func(repo.ui))

I suggest to split this patch because there are many hunks just changing
indent level.

Regards,

[Martin Geisler]

Angel Ezquerra <angel.e...@gmail.com> writes:

> # HG changeset patch
> # User Angel Ezquerra <angel.e...@gmail.com>
> # Date 1381344134 -7200
> # Wed Oct 09 20:42:14 2013 +0200
> # Node ID 7bf3715d98a4564f4fe2299096660ebdc30e84e4
> # Parent a9b7bcd86ac1937ffd825c9fb63ecf89f994e266
> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
>
> Issue #3373 suggests that it would be nice to be able to customize the
> changeset link on the revpanel so that it takes you go a web page for
> example.

Nice, I just tested it and it works just like expected!

Implementation details aside (which I don't know much about), I think
I've noticed a usability problem over the last month where I've been
using my hacky version at work.

The problem is that I have a hard time getting hold of a short node ID.
After making the "{rev} ({node|short})" part a link, I can no longer
double-click the short node ID in that string.

I need short node IDs several time per day: I reference commits in chat
messages, I reference them in commit messages and I reference them in
bug reports. The Copy Hash right-click option is not very useful since
it copies the long node ID which you don't want to put into human
readable text.

Maybe my best option would be to find the code that used to put the
short node ID of the selected commit into the Primary X clipboard
selection (the one you paste with the middle mouse button). I think I
was the last user of that feature before it was remove a while ago.

--
Martin Geisler

[Yuya Nishihara]

On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
> The problem is that I have a hard time getting hold of a short node ID.
> After making the "{rev} ({node|short})" part a link, I can no longer
> double-click the short node ID in that string.
>
> I need short node IDs several time per day: I reference commits in chat
> messages, I reference them in commit messages and I reference them in
> bug reports. The Copy Hash right-click option is not very useful since
> it copies the long node ID which you don't want to put into human
> readable text.
>
> Maybe my best option would be to find the code that used to put the
> short node ID of the selected commit into the Primary X clipboard
> selection (the one you paste with the middle mouse button). I think I
> was the last user of that feature before it was remove a while ago.

a) add config to switch "Copy Hash" menu to copy short node id

b) add config to copy short hash to X11 selection on revision selected
as in hgtk?

c) ...?

(a) is simple, but needs two taps to achieve. (b) seems a bit hackish.

Any suggestions?

[Martin Geisler]

Yuya Nishihara <yu...@tcha.org> writes:

> On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
>> The problem is that I have a hard time getting hold of a short node
>> ID. After making the "{rev} ({node|short})" part a link, I can no
>> longer double-click the short node ID in that string.
>>
>> I need short node IDs several time per day: I reference commits in
>> chat messages, I reference them in commit messages and I reference
>> them in bug reports. The Copy Hash right-click option is not very
>> useful since it copies the long node ID which you don't want to put
>> into human readable text.
>>
>> Maybe my best option would be to find the code that used to put the
>> short node ID of the selected commit into the Primary X clipboard
>> selection (the one you paste with the middle mouse button). I think I
>> was the last user of that feature before it was remove a while ago.
>
> a) add config to switch "Copy Hash" menu to copy short node id

As you mention, it's slower for to right-click and select "Copy Hash"
than to click a revision.

> b) add config to copy short hash to X11 selection on revision selected
> as in hgtk?
>
> c) ...?
>
> (a) is simple, but needs two taps to achieve. (b) seems a bit hackish.

Option (b) doesn't seem hacky to me -- the primary selection is normally
set implicitly when you highlight text or select something so this use
case matches quite well IMO.

Discovery of the feature is a problem: just like many users probably
haven't discovered that selecting text copies it, many users won't
notice that clicking a revision copies the hash. For now I'll just
suggest updating the documentation to mention it. Please see the patch
below:

# HG changeset patch
# User Martin Geisler <mar...@geisler.net>
# Date 1381671899 -7200
# Sun Oct 13 15:44:59 2013 +0200
# Node ID 1547d4a0822faf479ef9c9fdd9bb9af7ecdebe92
# Parent b916b9046cc956930406051133c8ad93adbfcc92
repoview: set X primary selection to short node ID on revClicked

The X Windows system supports more than one clipboard (or selections)
as they are called. The two main selections are:

* Clipboard: Stable, changed explicitly by the user when he copies
something, such as by selecting "Copy" from a menu or a toolbar.
Pasted when the user clicks "Paste" in an application.

* Primary: Ephemeral, changed implicitly when text selected with the
mouse or when data is clicked. Pasted with the middle mouse button
or Shift+Insert.

This makes TortoiseHg update the primary selection when a revision is
clicked. This makes it very fast to get hold of the short node ID for
pasting into other systems such as chats, bug trackers, emails, etc.

References:

http://www.jwz.org/doc/x-cut-and-paste.html
http://en.wikipedia.org/wiki/X_Window_selection#Clipboard
http://pyqt.sourceforge.net/Docs/PyQt4/qclipboard.html#Mode-enum

diff --git a/doc/source/workbench.txt b/doc/source/workbench.txt
--- a/doc/source/workbench.txt
+++ b/doc/source/workbench.txt
@@ -384,6 +384,12 @@

:guilabel:`Copy hash`
Copy current revision's full hash to the clipboard.
+
+ Under X11, the short changeset hash is automatically
+ copied to the primary selection when the revision is
+ clicked, you paste it by pressing the middle mouse
+ button.
+
:guilabel:`Modify history`
:guilabel:`QGoto`
Push/pop patches upto this one
diff --git a/tortoisehg/hgqt/repoview.py b/tortoisehg/hgqt/repoview.py
--- a/tortoisehg/hgqt/repoview.py
+++ b/tortoisehg/hgqt/repoview.py
@@ -209,6 +209,10 @@

def revClicked(self, index):
rev = self.revFromindex(index)
+ if rev is not None:
+ clip = QApplication.clipboard()
+ if clip.supportsSelection():
+ clip.setText(str(self.repo[rev]), mode=QClipboard.Selection)
if QApplication.keyboardModifiers() & Qt.AltModifier:
self.revisionAltClicked.emit(rev)
else:

--
Martin Geisler

[Yuya Nishihara]

On Sun, 13 Oct 2013 15:45:48 +0200, Martin Geisler wrote:
> > On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
> > b) add config to copy short hash to X11 selection on revision selected
> > as in hgtk?
> >
> > c) ...?
> >
> > (a) is simple, but needs two taps to achieve. (b) seems a bit hackish.
>
> Option (b) doesn't seem hacky to me -- the primary selection is normally
> set implicitly when you highlight text or select something so this use
> case matches quite well IMO.

Okay, Unix people would think in that way.

> Discovery of the feature is a problem: just like many users probably
> haven't discovered that selecting text copies it, many users won't
> notice that clicking a revision copies the hash.

Yep, I grepped hgtk code to understand what you mean.

> For now I'll just
> suggest updating the documentation to mention it. Please see the patch
> below:

Thanks, the patch looks good to me.

> # HG changeset patch
> # User Martin Geisler <mar...@geisler.net>
> # Date 1381671899 -7200
> # Sun Oct 13 15:44:59 2013 +0200
> # Node ID 1547d4a0822faf479ef9c9fdd9bb9af7ecdebe92
> # Parent b916b9046cc956930406051133c8ad93adbfcc92
> repoview: set X primary selection to short node ID on revClicked

[...]

> def revClicked(self, index):
> rev = self.revFromindex(index)
> + if rev is not None:
> + clip = QApplication.clipboard()
> + if clip.supportsSelection():
> + clip.setText(str(self.repo[rev]), mode=QClipboard.Selection)

"if clip.supportsSelection()" seems not necessary. I'll remove it.

https://qt.gitorious.org/qt/qt/source/0726127285413829f58618b5b82fb3e2da0c3a74:src/gui/kernel/qclipboard_win.cpp#L261

Regards,

[Yuya Nishihara]

and removed 'mode=' for compatibility with PyQt 4.6 on Cent OS 6.3.

I'll push it tomorrow.

Regards,

[Martin Geisler]

Yuya Nishihara <yu...@tcha.org> writes:

> On Sun, 13 Oct 2013 15:45:48 +0200, Martin Geisler wrote:
>> > On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
>> > b) add config to copy short hash to X11 selection on revision selected
>> > as in hgtk?
>> >
>> > c) ...?
>> >
>> > (a) is simple, but needs two taps to achieve. (b) seems a bit hackish.
>>
>> Option (b) doesn't seem hacky to me -- the primary selection is normally
>> set implicitly when you highlight text or select something so this use
>> case matches quite well IMO.
>
> Okay, Unix people would think in that way.

:-)

>> Discovery of the feature is a problem: just like many users probably
>> haven't discovered that selecting text copies it, many users won't
>> notice that clicking a revision copies the hash.
>
> Yep, I grepped hgtk code to understand what you mean.
>
>> For now I'll just suggest updating the documentation to mention it.
>> Please see the patch below:
>
> Thanks, the patch looks good to me.
>
>> # HG changeset patch
>> # User Martin Geisler <mar...@geisler.net>
>> # Date 1381671899 -7200
>> # Sun Oct 13 15:44:59 2013 +0200
>> # Node ID 1547d4a0822faf479ef9c9fdd9bb9af7ecdebe92
>> # Parent b916b9046cc956930406051133c8ad93adbfcc92
>> repoview: set X primary selection to short node ID on revClicked
> [...]
>> def revClicked(self, index):
>> rev = self.revFromindex(index)
>> + if rev is not None:
>> + clip = QApplication.clipboard()
>> + if clip.supportsSelection():
>> + clip.setText(str(self.repo[rev]), mode=QClipboard.Selection)
>
> "if clip.supportsSelection()" seems not necessary. I'll remove it.
>
> https://qt.gitorious.org/qt/qt/source/0726127285413829f58618b5b82fb3e2da0c3a74:src/gui/kernel/qclipboard_win.cpp#L261

So Qt ignores the unsupported mode, nice.

I've only tested under Linux and wondered what would happen if the mode
wasn't supported. So I tested quickly with QClipboard.FindBuffer instead
of QClipboard.Selection to see what happens when an unsupported mode is
used. I got a warning like this:

QClipboard::setMimeData: unsupported mode '2'

I assumed a similar warning will be printed for QClipboard.Selection on
Windows and OS X, which is why I added the check.

--
Martin Geisler

[Angel Ezquerra]

On Sun, Oct 13, 2013 at 1:08 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> On Sun, 13 Oct 2013 01:25:17 +0200, Angel Ezquerra wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <angel.e...@gmail.com>
>> # Date 1381344134 -7200
>> # Wed Oct 09 20:42:14 2013 +0200
>> # Node ID 7bf3715d98a4564f4fe2299096660ebdc30e84e4
>> # Parent a9b7bcd86ac1937ffd825c9fb63ecf89f994e266
>> revpanel: add linkpattern config to customize the target of cset links (fixes #3373)
>>
>> Issue #3373 suggests that it would be nice to be able to customize the changeset
>> link on the revpanel so that it takes you go a web page for example.
>>
>> In order to do so, we introduce a tortoisehg/linkpattern config setting, which
>> is a "pattern string" that serves as a pattern to generate the actual link URL.
>>
>> The linkpattern uses a mercurial "template like" syntax that currently accepts
>> two template variables: {node}, which is replaced by the 40 digit revision id,
>> and {rev} which is the revision number.
>
> Actually {node} is replaced by the short hash. ;)

True. In that case we cannot use {node} but we must use {node|short}
instead, and for now at least not support {node} on its own.

>> +def create_markup_func(ui):
>> + def link_markup(revnum, revid, enable=True, linkpattern=None):
>> mrevid = revid_markup('%s (%s)' % (revnum, revid))
>> if not enable:
>> return mrevid
>> - link = 'cset:%s' % revid
>> + if not linkpattern:
>> + linkpattern = 'cset:{node}'
>> + link = linkpattern.replace('{node}', revid).replace('{revnum}', revnum)
>
> It should be '{rev}'.

Sure!

>> + if item == 'cset':
>> + linkpattern = ui.config('tortoisehg', 'linkpattern', None)
>> + link = linkpattern is not None
>> else:
>> - csets.append(revline_markup(*cset))
>> - return csets
>> - raise csinfo.UnknownItem(item)
>> + linkpattern = None
>> + link = True
>
> Probably you can merge "link" (boolean) and "linkpattern" like this:
>
> if item == 'cset':
> linkpattern = ui.config('tortoisehg', 'linkpattern')
> else:
> linkpattern = 'cset:{node}'

OK.

> FWIW, it needs more specific name. How about 'tortoisehg.changesetlink'
> or 'tortoisehg.changeset.link'? There's already 'tortoisehg.issue.link'
> setting.

I agree. I even thought about it but I had not decided on a better
name when I sent this patch. 'tortoisehg.changeset.link' seems fine to
me.

I had also thought that this seems somewhate related to the issue
links. If you feel the same maybe we could place this setting on the
"Issue Tracking" settings panel? I know it is not entirely obvious but
it seems a better place than most, and I really do not want to put it
in the Workbench panel.

If you agree I can make it as a separate patch once this one is done.

>> + if isinstance(value, basestring):
>> + return revid_markup(value)
>> + return revline_markup(link=link, linkpattern=linkpattern, *value)
>> + elif item in ('parents', 'children', 'precursors', 'successors'):
>> + csets = []
>> + for cset in value:
>> + if isinstance(cset, basestring):
>> + csets.append(revid_markup(cset))
>> + else:
>> + csets.append(revline_markup(*cset))
>> + return csets
>> + raise csinfo.UnknownItem(item)
>> + return markup_func
>>
>> def RevPanelWidget(repo):
>> '''creates a rev panel widget and returns it'''
>> custom = csinfo.custom(data=data_func, label=label_func,
>> - markup=markup_func)
>> + markup=create_markup_func(repo.ui))
>
> I suggest to split this patch because there are many hunks just changing
> indent level.

OK. I will make a first patch that just adds a create_markup_func that
binds markup_func to the repo.ui, but which does not change the
functionality at all, and then a second one which actually does the
change. This means that for a single revision the bound repo.ui will
go unused, but I don't think that is a big deal, ok? Or did you have
something else in mind?

Cheers,

Angel

[Angel Ezquerra]

On Sun, Oct 13, 2013 at 2:36 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
>> The problem is that I have a hard time getting hold of a short node ID.
>> After making the "{rev} ({node|short})" part a link, I can no longer
>> double-click the short node ID in that string.
>>
>> I need short node IDs several time per day: I reference commits in chat
>> messages, I reference them in commit messages and I reference them in
>> bug reports. The Copy Hash right-click option is not very useful since
>> it copies the long node ID which you don't want to put into human
>> readable text.

I must say that I have the same issue than Martin, i.e. I want to copy
the short version of the node ID way more often than the full version.
I have considered many times sending a patch that would let you
configure what "Copy hash" does.

BTW, my current solution, which I don't know if Martin is aware of, is
to simply click on the hash column on the revision history widget and
then hit Ctrl+C. This works fine but I'd also like to be able to use
the context menu.

>> Maybe my best option would be to find the code that used to put the
>> short node ID of the selected commit into the Primary X clipboard
>> selection (the one you paste with the middle mouse button). I think I
>> was the last user of that feature before it was remove a while ago.
>
> a) add config to switch "Copy Hash" menu to copy short node id
>
> b) add config to copy short hash to X11 selection on revision selected
> as in hgtk?
>
> c) ...?
>
> (a) is simple, but needs two taps to achieve. (b) seems a bit hackish.
>
> Any suggestions?

I wouldn't be against (a). In fact, as I said, I considered sending
such patch myself in the past.

On windows, where thre is a single clipboard, perhas we could make it
so that if you hold Shift while you click "Copy Hash" then the short
node ID is copied? It is not super common nor very discoverable but
I've seen some programs do such things in the past...

Cheers,

Angel

[Martin Geisler]

Angel Ezquerra <angel.e...@gmail.com> writes:

> On Sun, Oct 13, 2013 at 2:36 PM, Yuya Nishihara <yu...@tcha.org> wrote:
>> On Sun, 13 Oct 2013 14:03:43 +0200, Martin Geisler wrote:
>>> The problem is that I have a hard time getting hold of a short node ID.
>>> After making the "{rev} ({node|short})" part a link, I can no longer
>>> double-click the short node ID in that string.
>>>
>>> I need short node IDs several time per day: I reference commits in chat
>>> messages, I reference them in commit messages and I reference them in
>>> bug reports. The Copy Hash right-click option is not very useful since
>>> it copies the long node ID which you don't want to put into human
>>> readable text.
>
> I must say that I have the same issue than Martin, i.e. I want to copy
> the short version of the node ID way more often than the full version.
> I have considered many times sending a patch that would let you
> configure what "Copy hash" does.
>
> BTW, my current solution, which I don't know if Martin is aware of, is
> to simply click on the hash column on the revision history widget and
> then hit Ctrl+C. This works fine but I'd also like to be able to use
> the context menu.

Oh, that's a quite simple and obvious solution, but I somehow never
thought of it :) Probably because I'm mostly using the middle-mouse
paste action in Linux.

--
Martin Geisler

[Yuya Nishihara]

On Sun, 13 Oct 2013 18:26:32 +0200, Martin Geisler wrote:
> > "if clip.supportsSelection()" seems not necessary. I'll remove it.
> >
> > https://qt.gitorious.org/qt/qt/source/0726127285413829f58618b5b82fb3e2da0c3a74:src/gui/kernel/qclipboard_win.cpp#L261
>
> So Qt ignores the unsupported mode, nice.
>
> I've only tested under Linux and wondered what would happen if the mode
> wasn't supported. So I tested quickly with QClipboard.FindBuffer instead
> of QClipboard.Selection to see what happens when an unsupported mode is
> used. I got a warning like this:
>
> QClipboard::setMimeData: unsupported mode '2'
>
> I assumed a similar warning will be printed for QClipboard.Selection on
> Windows and OS X, which is why I added the check.

I don't know why, but only X11 implementation shows this warning.

Regards,