Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a generic class with different type arguments is rendered as the same class symbol #4762

Open
ksilverwall opened this issue Aug 22, 2023 · 4 comments
Assignees
Labels
Graph: Class Status: Approved Is ready to be worked on Status: Awaiting PR Type: Bug / Error Something isn't working or is incorrect

Comments

@ksilverwall
Copy link
Contributor

ksilverwall commented Aug 22, 2023

Description

INPUT a generic class with different type arguments, ex class Shape~T1~ and class Shape~T2~

classDiagram
  class Shape~T1~
  class Shape~T2~

rendered as a same object

classDiagram
  class Shape~T1~
  class Shape~T2~
Loading

Generally, classes with different types should be interpreted as different types.

Steps to reproduce

render this code

classDiagram
  class Shape~T1~
  class Shape~T2~

Screenshots

No response

Code Sample

No response

Setup

  • Mermaid version:
  • Browser and Version: [Chrome, Edge, Firefox]

Suggested Solutions

No response

Additional Context

I will discuss specifically how it can be useful.
I want to write diagram like this. (_xxx_ means Generics)

classDiagram
  class CompositorHandler {
    RequestQueue: Queue_CompositorMessage_
  }
  class ConverterHandler {
    MainQueue: Queue_ConverterMessage_
    SubQueue: Queue_ConverterMessage_
  }
  class Queue_T_ {
    <<interface>>
    Pop(): T, error
		Push(): error
  }
  class Queue_CompositorMessage_ {
    <<interface>>
    Pop(): T, error
		Push(): error
  }
  class Queue_ConverterMessage_ {
    <<interface>>
    Pop(): T, error
		Push(): error
  }
  class CompositorSQS
  class ConverterSQS

  Queue_T_ <|-- Queue_CompositorMessage_: T=CompositorMessage
  Queue_T_ <|-- Queue_ConverterMessage_: T=ConverterMessage
  CompositorHandler *-- Queue_CompositorMessage_
  ConverterHandler *-- Queue_ConverterMessage_
  Queue_CompositorMessage_ <|-- CompositorSQS
  Queue_ConverterMessage_ <|-- ConverterSQS
Loading
@ksilverwall ksilverwall added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Aug 22, 2023
@tomperr
Copy link
Contributor

tomperr commented Aug 22, 2023

This is an interesting feature. However, this might change how we identify classes with generics.
For example, with your example, to add a relationship we would do

classDiagram
    class Shape~T1~
    class Shape~T2~
    Shape~T1~ --> Shape~T2~

Currently, as we handle classes with generics that do not have the same classnames, we do it that way

classDiagram
    class A~T1~
    class B~T2~
    A --> B

In the relationship, we identify the classes by their classname only, not by their classname + their type.

So, to make this feature possible, we must force the user to specify the classnames + the types in relationship (as in the first example), OR we must accept both : with types if needed (like in the first example), without if not (like in the second example). The first option would be a breaking change.

@sidharthv96
Copy link
Member

The first option would be a breaking change.

Which we cannot have in syntax. So we should go with the 2nd option.

@sidharthv96 sidharthv96 added Status: Approved Is ready to be worked on Graph: Class Area: Development Status: Awaiting PR and removed Status: Triage Needs to be verified, categorized, etc labels Aug 25, 2023
@jgreywolf
Copy link
Contributor

I will look into this, as I am going through a bunch of stuff in class diagrams right now. But I am concerned that the solution put in place to handle class generics many years ago might make this a little difficult...

@jgreywolf jgreywolf added roadmap items to add to roadmap for auto workflow and removed roadmap items to add to roadmap for auto workflow labels Nov 16, 2023
@jgreywolf jgreywolf self-assigned this Dec 22, 2023
@jgreywolf
Copy link
Contributor

I am sorry for the LONG delay - but I am now actively working on an update to the class diagram syntax that will address this, and other, issues.

Should have something within a couple of days :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph: Class Status: Approved Is ready to be worked on Status: Awaiting PR Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants