Roundup Tracker - Issues

Issue 2551108

classification
Hyperlink replaced multiple times
Type: behavior Severity: normal
Components: User Interface Versions: 2.0.0
process
Status: fixed fixed
:
: rouilj : ced, rouilj
Priority: :

Created on 2021-01-07 16:37 by ced, last changed 2021-03-13 16:10 by rouilj.

Messages
msg7046 Author: [hidden] (ced) Date: 2021-01-07 16:37
When using Markdown format if the message content a markdown link to an existing issue, the result is that we have a link to the issue with a text like `[issuexyz](issuexyz)`.
This is because call to `hyper_re.sub` call the method for every non-overlapping occurrence of the pattern.
I think a possible option is to use rewrite the regexp to not overlap (as suggested by https://stackoverflow.com/questions/5616822/python-regex-find-all-overlapping-matches#comment81721360_5616822).
I think also that the problem exist for other format using the same regexp.
msg7047 Author: [hidden] (rouilj) Date: 2021-01-10 02:14
Hi Cedric:

Sorry, I am lost here. What I understand is that the input text
is something like:

   issuexyz

the output between the a, /a tags is `[issuexyz](issuexyz)` or is
each of [issuexyz] and (issuezyz) a separate a tag?

Your url reference looks like it is a technique to return
multiple matches reusing subelements that have already
been matched. E.G. when matching three digits
for 123456 return:

   123 234 345 456

and not

   123 456
 
which is I don't see as applicable here.
msg7048 Author: [hidden] (ced) Date: 2021-01-10 10:04
On 2021-01-10 02:14, John Rouillard wrote:
> Sorry, I am lost here. What I understand is that the input text
> is something like:
> 
>    issuexyz
> 
> the output between the a, /a tags is `[issuexyz](issuexyz)` or is
> each of [issuexyz] and (issuezyz) a separate a tag?

No, the input is:

    [issuexyz](http://bugs.example.com/issuexyz)

And the HTML output is:

    <a href="http://bugs.example.com/issuexyz">[issuexyz](issuexyz)</a>

> Your url reference looks like it is a technique to return
> multiple matches reusing subelements that have already
> been matched. E.G. when matching three digits
> for 123456 return:
> 
>    123 234 345 456
> 
> and not
> 
>    123 456
>  
> which is I don't see as applicable here.

Maybe but the problem is that the regexp used overlaps so re.sub call
for the smallest matching but indeed we need that it calls for the
longest matching substring.
msg7049 Author: [hidden] (rouilj) Date: 2021-01-11 00:25
Ok, that's odd.

Is:

   http://bugs.example.com/issuexyz

some third party tracker? It should be the case that typing:

   issuexyz

should result in a hyperlink to issuexyz on the current tracker IIRC.
If the reference is to the local issue then I think the answer is
don't use markdown for that link.

However, what if you want to reference an issue on a third party 
tracker?

Do you know is this is due to a bad interaction between: 
cgi/templating.py:c_hyper_repl_markdown
and StringHTMLProperty::markdown? I assume that's where the call to 
self.hyper_re.sub that you originally referenced occurs.
msg7050 Author: [hidden] (ced) Date: 2021-01-11 10:34
On 2021-01-11 00:25, John Rouillard wrote:
> Is:
> 
>    http://bugs.example.com/issuexyz
> 
> some third party tracker?

No.

> It should be the case that typing:
> 
>    issuexyz
> 
> should result in a hyperlink to issuexyz on the current tracker IIRC.
> If the reference is to the local issue then I think the answer is
> don't use markdown for that link.

Yes indeed. But here it is users that are mixing both way and try to do
their best.

> However, what if you want to reference an issue on a third party 
> tracker?

It works as long as the id does not exist in the current tracker.

> Do you know is this is due to a bad interaction between: 
> cgi/templating.py:c_hyper_repl_markdown
> and StringHTMLProperty::markdown? I assume that's where the call to 
> self.hyper_re.sub that you originally referenced occurs.

What happens is that _hyper_repl_markdown is called first for the
substring: `issuexyz` which replace the string by
`[[issuexyz](issuexyz)](http://bugs.example.com/issuexyz)`.

So I guess for the `id` match group, we also need to test if it is not
surrounded by `[…]` like we do for 'url' and 'email' for `<…>`.
msg7097 Author: [hidden] (rouilj) Date: 2021-03-12 06:50
Hi Cedric:

Check out rev 6f89cdc7c938. I think I fixed your reported cases:

  [issue1](issue1)
  [issue1](https://example.com/issue1)

I also tested to make sure that:

  [issue1] (https://example.com/issue1)

(incorrect markdown syntax) did replace the first issue1 with
a link. Note that mistune does process the url into a link.
The other two back ends just leave it as is.

I was not able to reproduce your test case where the input:

     [issue1](https://example.com/issue1)

tried to turn the issue1 in https://example.com/issue1 into a link.
From the regexp that matches designators (aka items) I don't
see why the issue1 isn't being returned but.....

Can you take a look and if it solves your issue please close this 
ticket.

Thanks.

-- rouilj

P.S. rev 316c2c32dace is needed to fix some test cases broken by my 
changes to the test harness.
msg7103 Author: [hidden] (ced) Date: 2021-03-13 11:12
It seems to solve it but I can not change the status.
msg7104 Author: [hidden] (rouilj) Date: 2021-03-13 16:00
Hi Cedric:

I have closed the issue.

I'll have to take a look at the permissions structure here.

I'll tweak the permissions. Currently users are allowed to change 
everything in the classification box and the nosy list.

I think allowing Users to change status so they can self-close
issues make sense as well.

Have a great weekend and thanks for testing.

-- rouilj
msg7105 Author: [hidden] (rouilj) Date: 2021-03-13 16:10
Also Cedric, I added the Developer role to your ced account. That
will let you change status, resolution etc.

I have changed the schema so that users can modify the status, but
there are a few changes that have to be pushed to production. So the
update won't occur soon.

Have a great day.
History
Date User Action Args
2021-03-13 16:10:35rouiljsetmessages: + msg7105
2021-03-13 16:00:09rouiljsetstatus: pending -> fixed
resolution: fixed
messages: + msg7104
2021-03-13 11:12:35cedsetmessages: + msg7103
2021-03-12 06:50:26rouiljsetstatus: new -> pending
assignee: rouilj
messages: + msg7097
2021-01-11 10:34:03cedsetmessages: + msg7050
2021-01-11 00:25:59rouiljsetmessages: + msg7049
2021-01-10 10:04:03cedsetmessages: + msg7048
2021-01-10 02:14:23rouiljsetnosy: + rouilj
messages: + msg7047
2021-01-07 16:37:14cedcreate