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

Allow underscore as an "anonymous indentifier" #6073

Open
Hi-Angel opened this issue Nov 21, 2024 · 12 comments
Open

Allow underscore as an "anonymous indentifier" #6073

Hi-Angel opened this issue Nov 21, 2024 · 12 comments
Labels
Graph: Flow Status: Pending Is not to be executed as it currently is Type: Enhancement New feature or request

Comments

@Hi-Angel
Copy link

Proposal

Not every block in the diagram has to be referenced. For example, given this one:

flowchart TD
   subgraph FOOES
       FOO1["Foo description"]
       FOO2["Foo description"]
   end

   subgraph BARS
       BAR1["Bar description"]
       BAR2["Bar description"]
   end

   FOOES --> BARS

FOO1, FOO2, BAR1 and BAR2 are unused. So reading the sources would be simpler if the variables weren't created, because it becomes immediately clear which parts don't have any references to them.

In modern programming languages that's usually achieved by replacing the identifier name to a underscore _. However when doing the same to the diagram above, almost every block just disappears from the diagram:

flowchart TD
   subgraph FOOES
       _["Foo description"]
       _["Foo description"]
   end

   subgraph BARS
       _["Bar description"]
       _["Bar description"]
   end

   FOOES --> BARS
flowchart TD
   subgraph FOOES
       _["Foo description"]
       _["Foo description"]
   end

   subgraph BARS
       _["Bar description"]
       _["Bar description"]
   end

   FOOES --> BARS
Loading

Example

flowchart TD
   subgraph FOOES
       _["Foo description"]
       _["Foo description"]
   end

   subgraph BARS
       _["Bar description"]
       _["Bar description"]
   end

   FOOES --> BARS

Screenshots

No response

@Hi-Angel Hi-Angel added Status: Triage Needs to be verified, categorized, etc Type: Enhancement New feature or request labels Nov 21, 2024
@jgreywolf
Copy link
Contributor

jgreywolf commented Nov 23, 2024

@Hi-Angel Even if those nodes are not being referenced elsewhere, there may come a time when they are. Mermaid does not know if they are going to be referenced later, so all nodes need some sort of ID. If there are two nodes defined with the same ID, the last one will overwrite any of the ones prior, so you will only see one box.

I dont think it makes sense given the overall structure of the syntax to have "anonymous" boxes. They need an ID in order to be rendered

@jgreywolf jgreywolf added Status: Pending Is not to be executed as it currently is Graph: Flow Status: Awaiting Reply Close after 30 days Close issue if no response after 30 days and removed Status: Triage Needs to be verified, categorized, etc labels Nov 23, 2024
@Hi-Angel
Copy link
Author

@Hi-Angel Even if those nodes are not being referenced elsewhere, there may come a time when they are. Mermaid does not know if they are going to be referenced later, so all nodes need some sort of ID

I'm not sure I understand. If "time comes that the node become referenced", this means the user has to replace _ with a meaningful name. Till that happened, we can assume the node is not referenced from anywhere.

@Hi-Angel
Copy link
Author

If there are two nodes defined with the same ID, the last one will overwrite any of the ones prior, so you will only see one box.

As a side-note, I think this warrants a warning, because if a user has overwritten one node with another, that implies the first node could as well not exist, which in turn implies there's some sort of mistake on the user's part: either they created an unnecessary node, or they accidentally created a name that overlaps with existing one.

@Hi-Angel
Copy link
Author

@jgreywolf hello, can you please remove tags "Awaiting for Reply" and "Close after 30 days" since they're no longer relevant

@jgreywolf jgreywolf removed Status: Awaiting Reply Close after 30 days Close issue if no response after 30 days labels Nov 29, 2024
@jgreywolf
Copy link
Contributor

@sidharthv96 @knsv Can one of you look at this and make a recommendation?

@sidharthv96
Copy link
Member

In modern programming languages that's usually achieved by replacing the identifier name to a underscore _.

We cannot have multiple identifiers with a single _, this is the same behaviour with mermaid.
image

I do understand your requirement, and one way to handle it would be to assign a random/sequential id for each _ node that we encounter. But, doing that would break existing diagrams, which is against one of our core principles that we don't break existing diagrams.

Adding a warning is definitely possible, when encountering duplicates. PRs are open for that, and any other backwards-compatible approach to allow _s.

@Hi-Angel
Copy link
Author

Hi-Angel commented Dec 2, 2024

this is the same behaviour with mermaid.

Sorry, isn't this project the mermaid? Is there another "mermaid" project?

I do understand your requirement, and one way to handle it would be to assign a random/sequential id for each _ node that we encounter. But, doing that would break existing diagrams, which is against one of our core principles that we don't break existing diagrams.

I presume here you're talking about how would that be handled internally by mermaid-js. I don't see how that would break existing diagrams. The side-effect from adding such feature (that would influence existing diagrams) is that if someone named two elements with _, they were getting overwritten before, but now they would both get rendered. But as I elaborated in this comment, I don't see why someone would legitimately do that; and then a person needs not just "to overwrite an element with same id", but to overwrite an element by naming them exactly _.

That is, for an existing diagram to break by such feature you need someone to be naming more than one element with underscore _, which doesn't look realistic to me.

@Hi-Angel
Copy link
Author

Hi-Angel commented Dec 2, 2024

…now that I posted, my another hypothesis that you meant to say that if you handle the feature by generating an internal ID by doing an increment, such ID may collide with an existing ID. In that case the solution seems simple: just peek at the list of IDs before assigning a new name to check that such name doesn't exist. If you save existing ID's in a hashset or hashtable, then search is a O(1) operation, so very fast. Also, rather than increment, you can randomly generate the name.

@sidharthv96
Copy link
Member

Sorry, isn't this project the mermaid? Is there another "mermaid" project?

image

The screenshot I posted was of TypeScript, I was pointing out that programming languages also does not allow multiple _s in the same context.


I presume here you're talking about how would that be handled internally by mermaid-js.

Yup

I don't see how that would break existing diagrams.

By break, I mean render differently. They might've used the _ id for styling, or adding classes, or edges, etc, so we can't have a definite answer on how to handle some cases.

eg:

The following chart would be rendered differently, if we started attaching internal IDs for every _ node we see.

flowchart
_["a"]
_ --> b

@Hi-Angel
Copy link
Author

Hi-Angel commented Dec 2, 2024

The screenshot I posted was of TypeScript, I was pointing out that programming languages also does not allow multiple _s in the same context.

Ah, well, of course not every existing lang allows it, but many modern ones do. Offhand: Go, Rust, Haskell, Elisp, PureScript…

I don't see how that would break existing diagrams.

By break, I mean render differently. They might've used the _ id for styling, or adding classes, or edges, etc, so we can't have a definite answer on how to handle some cases.

eg:

The following chart would be rendered differently, if we started attaching internal IDs for every _ node we see.

flowchart
_["a"]
_ --> b

I see. You can avoid this problem by internally naming the first underscore _ id declaration just that _, and only assigning generated names to further ones. This ofc leaves the question of having multiple underscore identifiers in older diagram, but as noted above I don't see a legitimate case for such diagram.

@jgreywolf
Copy link
Contributor

This is not addressing the concern for existing diagrams, but something that I think Sid is saying here is that if you have multiple elements that are just _, and then we try to assign a "random" "unique" id to each of those, they will all be treated as separate entities, even if, in theory, some of them may be referring to the same object.

@Hi-Angel
Copy link
Author

Hi-Angel commented Dec 3, 2024

This is not addressing the concern for existing diagrams, but something that I think Sid is saying here is that if you have multiple elements that are just _, and then we try to assign a "random" "unique" id to each of those, they will all be treated as separate entities, even if, in theory, some of them may be referring to the same object.

As I noted above, I don't see a legitimate case for a diagram with multiple elements named the same. Do you know one? Besides, in this feature we're not talking about "any diagram with multiple elements with same name", but exactly about a diagram "with multiple elements named _", which sounds even less useful. I just don't see what legitimate case may break here.

Besides, is it even a documented behavior that repeating identifier declaration overwrites previous one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph: Flow Status: Pending Is not to be executed as it currently is Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants