plantuml / plantuml

Generate diagrams from textual description
https://plantuml.com
Other
9.73k stars 881 forks source link

Link in `ref` element not displayed #635

Closed redcatbear closed 2 years ago

redcatbear commented 2 years ago

Situation

When adding a link [[...]] in a ref element of a sequence chart, that link does not appear unless it is prefixed by regular text.

Since links are especially useful in ref sections, I think this should work.

Steps to reproduce

@startuml
Participant A
ref over A : [[link does not work]]
ref over A : [[link neither does this]] here
ref over A : see [[link this works]]
@enduml

Reproducibility

100%

Environment

The-Lum commented 2 years ago

Hello @redcatbear,

You have right. A possible workaround is to use link without label...

Here is an example with more combinatoric:

@startuml
title test of link on ref
note right A #palegreen: a link (without tooltip and label) works for all the ref area
ref over A : [[link]]
...
note right A #pink : a link on ref (with tooltip) does not work\n<b>[the tooltip is taken on the link!]
ref over A : [[link{link with tooltip}]]
ref over A : [[link{link with tooltip}]] with words on ref
note right A #palegreen: same link (with tooltip) work on on arrow's label
A -> A [[link{link with tooltip}]]: here is a OK example
...
note right A #pink : a link on ref (with label) does not work\n<b>[the label is taken on the link!]
ref over A : [[link with label]]
ref over A : [[link with label]] with words on ref
note right A #palegreen: same link (with label) work on arrow's label
A -> A [[link with label]]: here is a OK example
...
note right A #palegreen: a link works only for the words "this works"
ref over A : see [[link{with tooltip} this works]]
@enduml

@arnaudroques : This issue is only observed with link on ref with label or tooltip...

Regards.

gwilherm commented 2 years ago

Are ref supposed to manage links in a special way ? Because I found a fix for this issue but a removed some code that I don't understand what is it supposed to do. I believe that I may have break something :D

See the "fix" here : https://github.com/gwilherm/plantuml/commit/429f4b1598bc7f51f1f35ce963a422fd04750b81

image

The-Lum commented 2 years ago

@gwilherm: Good, but...

I liked the idea of setting the link at the beginning for the whole shape of the ref., as:

@startuml
title test of link on ref
ref over A : [[link]] (Link on the whole shape)\n with words on ref
ref over A : [[https://plantuml.com/link]] (Link on the whole shape)\n with words on ref
@enduml

Click to the image to go to the PlantUML Online server, then see the link in action...

An hackathon or a review of link management must be perhaps plan to improve that...

Regards.

The-Lum commented 2 years ago

Hello @gwilherm, @arnaudroques, and all,

After second analyse... In fact the better syntax (consistent with other link examples) for that (link on whole shape) is perhaps:

@startuml
title test of link on ref
ref over A [[link]] : (Link on the whole shape)\n with words on ref
ref over A [[https://plantuml.com/link]] : (Link on the whole shape)\n with words on ref
@enduml

Then perhaps change only the order of those lines and re-test your change: https://github.com/plantuml/plantuml/blob/b38adcfcdcf2b1f0f0c8424c2b51e3f7a75a6e3f/src/net/sourceforge/plantuml/sequencediagram/command/CommandReferenceOverSeveral.java#L70-L76

to:

new RegexLeaf("PARTS",
        "(([%pLN_.@]+|[%g][^%g]+[%g])([%s]*,[%s]*([%pLN_.@]+|[%g][^%g]+[%g]))*)"), //
RegexLeaf.spaceZeroOrMore(), //
new RegexOptional(new RegexLeaf("URL", "\\[\\[([^|]*)(?:\\|([^|]*))?\\]\\]")), //
RegexLeaf.spaceZeroOrMore(), //
new RegexLeaf(":"), //
new RegexLeaf("TEXT", "(.*)"), RegexLeaf.end());

If that can help. Regards.


Just for traceability; Here is a similar request here about the general issue:

gwilherm commented 2 years ago

Hello,

I agree that it would be more clear with link before :,

I also found one working pretty well : \[\[([^\s\{]*)(?:\s*\{(.*?)\})?(.*?)?\]\] That you can test here : https://regex101.com/r/jaD6G6/1

link-in-ref

Gwilherm.

The-Lum commented 2 years ago

Hello @gwilherm, and all,

About URL and regex topic, see also:

Regards.

The-Lum commented 2 years ago

In fact the big 🧹 'spring cleaning' will be to replace all pseudo matching URL regex on RegexLeaf("URL", ...) by UrlBuilder.getRegexp()!

Go to open an hackathon...

gwilherm commented 2 years ago

Ok next round with UrlBuilder :D

With this simple Regex:

RegexLeaf.spaceZeroOrMore(), //
new RegexLeaf(":"), //
RegexLeaf.spaceZeroOrMore(), //
new RegexOptional(new RegexLeaf("URL", "(\\[\\[.*?\\]\\])")), //
new RegexLeaf("TEXT", "(.*)"), RegexLeaf.end());

And the builder:

final UrlBuilder b = new UrlBuilder(null, UrlMode.STRICT);
Url u = null;
if(url != null) {
    u = b.getUrl(url);
}

This works pretty well !

arnaudroques commented 2 years ago

Ok next round with UrlBuilder :D

Thanks all for your tries! This makes indeed sense to use UrlBuilder here.

The reason why we don't right now is just because CommandReferenceOverSeveral class has been written a long time ago, even before we introduce UrlBuilder.

Since we are very lazy :-) , would you mind pushing a pull request ?

gwilherm commented 2 years ago

After 3 oopsies in my repo here it is : #976