plantuml / plantuml

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

nwdiag bug: group causes element to incorrectly connect to network #1301

Closed dgutson closed 1 year ago

dgutson commented 1 year ago

Given:

@startuml

nwdiag {
network net1 {
thing_1
}
network net2 {
thing_2
}
group uniqueNode {
  thing_1
}

}

@enduml

The following is rendered: image

Clearly, thing1 should not be connected to net2. This is correctly rendered using nwdiag: image

dgutson commented 1 year ago

Datum: if I write the group before the network definitions, the connections are correct, but descriptions, colors and <style> group are ignored.

dgutson commented 1 year ago

@arnaudroques sorry to tag you, any chance to take a look? this is a serious bug for us. At least an initial evaluation.

arnaudroques commented 1 year ago

@arnaudroques sorry to tag you, any chance to take a look? this is a serious bug for us. At least an initial evaluation.

We're going to have a look at it. It should be easy to fix. I'll post a message here where this will be ready.

dgutson commented 1 year ago

@arnaudroques sorry to tag you, any chance to take a look? this is a serious bug for us. At least an initial evaluation.

We're going to have a look at it. It should be easy to fix. I'll post a message here where this will be ready.

Thank you so much!! We created a network diagrams exporter for structurizr that we will open source soon, and this is key for the demo this Friday to get the publication clearance.

arnaudroques commented 1 year ago

Thank you so much!! We created a network diagrams exporter for structurizr that we will open source soon, and this is key for the demo this Friday to get the publication clearance.

Someday, you should tell us more about this.

We've done few tests but it should work better now.

You can also use last snapshot.

Tell us if it's not working for you!

dgutson commented 1 year ago

Thanks @arnaudroques , but the snapshots is not working well :( It is completely ignoring:

  1. the style:

    <style>
    nwdiagDiagram {
    group {
    BackGroundColor cadetblue
    #LineColor black
    #LineThickness 2.0
    #FontSize 11
    FontStyle bold
    #Margin 5
    #Padding 5
    }
    }
    </style>
  2. the attributes

    group mainNode {
    description = "mainNode "
    DB_Main_1 [color = "Red", description="<b>DB_Main</b>\n\nBackend DB\nCU DB\nXXX DB\nYYY DB"]

everything within [ ] gets ignored

My group definitions are after the network definitions, as shown in my original example.

All these use to work in 1.2023.1

dgutson commented 1 year ago

Example

arnaudroques commented 1 year ago

Example

Thanks for the example! It should work now.

Still one day to test :-)

dgutson commented 1 year ago

@arnaudroques thanks, network diagrams are now working as expected!

However, the snapshot is badly broken for other diagrams, are you running regression tests yet or tests already finished? I hope I won't need to file all those issues :(

arnaudroques commented 1 year ago

However, the snapshot is badly broken for other diagrams

Could you give an simple example? Thanks

dgutson commented 1 year ago

Unfortunately not unless I write an obfuscator. I spent an hour simplifying the network diagram of the example above. I'll try to come up with something.

arnaudroques commented 1 year ago

However, the snapshot is badly broken for other diagrams

We do have some non-regression tests for network diagrams, but on very simple diagrams. Yours seem to be more complex. When you talk about "other" diagrams, do you mean non-network diagrams? Or are you talking about some other diagram networks?

Maybe you are impacted by the namespace/package simplification (see https://plantuml.com/en/news ) ? In that case set separator none may help you.

dgutson commented 1 year ago

why such a breaking change :( Any way to change the default to be blackward compatible, or add a configuration file? Please note that these diagrams (not network ones) are generated by structurizr, so this should be modified by @simonbrowndotje by changing the exporter behavior depending on what plantUML version it would generate for, add a property or env var, really a hassle. I think that with this change, suddenly all plantUML users, not only structurizr users, will suddenly get their diagrams broken.

dgutson commented 1 year ago

(yes, writing set separator none at the top solves the problem bringing back the relationships)

dgutson commented 1 year ago

Without the set separator none: image

With set separator none (as generated in the previous version): image

arnaudroques commented 1 year ago

Please note that these diagrams (not network ones) are generated by structurizr, so this should be modified by @simonbrowndotje by changing the exporter behavior depending on what plantUML version it would generate for, add a property or env var

Note that previous PlantUML version should not be impacted by this set separator none so it can be set whatever is the version.

I think that with this change, suddenly all plantUML users, not only structurizr users, will suddenly get their diagrams broken.

Theoretically, only those that put some dot (".") in their package names. There is something strange in your case, but the image are too small and not readable: I don't get why relationships are impacted by this change. Probably something we should investigate. @simonbrowndotje Any idea?

dgutson commented 1 year ago

Indeed the names have dots in C4 to specify the scope when there is hierarchy (softwaresystem/container/component).

Sorry about the unreadable image (this was a cheap obfuscation), this is everything I could do until I can come up with a curated example.

simonbrowndotje commented 1 year ago

@simonbrowndotje Any idea?

Probably using an old version of the Structurizr export library ... I added support for set separator none 2 weeks ago:

dgutson commented 1 year ago

@arnaudroques @simonbrowndotje this was generated with latest structurizr-cli (docker) and latest plantUML snapshot (v1.2023.3beta5), a dynamic view (sequence diagram mode), the error is: image

I understand I should show the source of both dsl and plantUML, but at least from the error, can you spot what the error is?

simonbrowndotje commented 1 year ago

Here's a simplified example:

@startuml
set separator none

actor "User" as User
participant "A.B" as A.B
User -> A.B : Uses

@enduml

image

The-Lum commented 1 year ago

Hello all,

It seems due to the type of diagram, compare:

Actor diagram

@startuml
set separator none

actor "User" as User
'participant "A.B" as A.B
User -> A.B : Uses
@enduml

Sequence diagram

@startuml
'set separator none

actor "User" as User
participant "A.B" as A.B
User -> A.B : Uses
@enduml

That seems set separator, on Sequence diagram context, which occurs the error.

If that can help, Regards, Th.

arnaudroques commented 1 year ago

Here's a simplified example:

Thanks for the simplified example!

As @The-Lum said, if you try to draw a sequence diagram, set separator does not make sense, since there are no package/namespace in sequence diagram.

So sorry for not having been precise enough, but set separator none should be only added for class/object/usecase/deployment diagrams.

Does it help?

The-Lum commented 1 year ago

Does it help?

@arnaudroques: Yes.

But, now that will be a new wanted feature:

In this case, we could include this line set separator none on theme, configuration file, etc... without presuppose which diagram will be used by... (and especially without error for Sequence diagram...)

If I am clear... Regards.

arnaudroques commented 1 year ago

If I am clear...

Very clear! This has been implemented in last snapshot and on the online server.

dgutson commented 1 year ago

Just tested ~v1.2023.3beta7 and everything looks good. Please don't touch anything else :) release it.