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

Standardized diagram keywords #4542

Open
sidharthv96 opened this issue Jun 27, 2023 · 10 comments
Open

Standardized diagram keywords #4542

sidharthv96 opened this issue Jun 27, 2023 · 10 comments

Comments

@sidharthv96
Copy link
Member

sidharthv96 commented Jun 27, 2023

It would be great if we standardized diagram keywords, because some diagrams just have the actual keyword:

  • pie
  • timeline
  • gantt
  • mindmap
    ...etc.

And some have additional word:

  • quadrantChart
  • stateDiagram
  • erDiagram
  • sequenceDiagram
    ...etc.

I think this addition wouldn't really help; it's just making the keyword longer.

Originally posted by @Yokozuna59 in #4499 (comment)

@github-actions github-actions bot added the Status: Triage Needs to be verified, categorized, etc label Jun 27, 2023
@sidharthv96
Copy link
Member Author

sidharthv96 commented Jun 27, 2023

Needs to be discussed here separate.

This is the current detectors that we have, new changes should be made in the parser jison files, and the detectors, while maintaining backwards compatibility.

Current Proposed Remarks
graph graph
flowchart flow
stateDiagram state
journey journey
pie pie
gantt gantt
info info
requirement(Diagram) requirement
classDiagram-v2 class
classDiagram class
C4Context, C4Container, C4Component, C4Dynamic, C4Deployment c4 Should we have separate ones?
mindmap mindmap
sequenceDiagram sequence
erDiagram er
timeline timeline
quadrantChart quadrant
gitGraph git

@Yokozuna59
Copy link
Member

Yokozuna59 commented Jun 27, 2023

I think the proposed keywords look great, but a quick question about two-word keywords: should we also support the full version? Or the current ones are fine? i.e., user journey, entity relation.

I'm aware almost no one would use the full keyword, but just it's just a question came to my mind, and maybe we face in the future.


You have forgotten stateDiagram-v2, flowchart-elk.


For c4, we could add a keyword after c4 similar to direction in flowchart:

flowchart TB
...
c4 context
...

As far as I know, c4 doesn't have direction change, right?

EDIT: I'm not really sure about seperating them; it would show that they are separate diagrams. At the end, there would be only file for there all syntaxes. It would be great if we separated logic into different files similar to flowchart-elk, because it's hard to navigate right now.

@tomperr
Copy link
Contributor

tomperr commented Jun 27, 2023

I agree: keeping only c4 keyword seems way easier for the user, and splitting the different levels in dedicated files as we do for flowchart-elk would be much clearer for developers

@sidharthv96
Copy link
Member Author

I think the proposed keywords look great, but a quick question about two-word keywords: should we also support the full version? Or the current ones are fine? i.e., user journey, entity relation.

I'm aware almost no one would use the full keyword, but just it's just a question came to my mind, and maybe we face in the future.

In cases where we "need" to support 2 words, lowerCamelCase seems like the right choice, as adding a whitespace might complicate our internals.


You have forgotten stateDiagram-v2, flowchart-elk.

I'm not certain how we should handle those. Especially elk.
For stateDiagram-v2, we can simply use state, as any new diagrams using the state keyword will be v2, and older diagrams with stateDiagram-v2 will continue working as the changes MUST be backwards compatible.

But flowchart ELK, is simply a different layout engine, so maybe we could support elk as an optional keyword like we support orientation.

flow [elk], so both flow elk and flow will be supported.


I'm not much familiar with C4 diagrams, but c4 context|container|component... seems like a suitable option.

@sidharthv96
Copy link
Member Author

Change Current Proposed Remarks
x graph flow graph == flow
flowchart flow
x flowchart-elk flow elk elk is an optional modifier on top of flow.
stateDiagram state
stateDiagram-v2 state
journey journey
pie pie
gantt gantt
info info
requirement(Diagram) requirement
classDiagram-v2 class
classDiagram class
x C4Context, C4Container, C4Component, C4Dynamic, C4Deployment c4 context|container|component|dynamic|deployment The specifier will be mandatory
mindmap mindmap
sequenceDiagram sequence
erDiagram er
timeline timeline
quadrantChart quadrant
gitGraph git

@Yokozuna59
Copy link
Member

In cases where we "need" to support 2 words, lowerCamelCase seems like the right choice, as adding a whitespace might complicate our internals.

I wasn't referring to adding whitespace, just listing them out, but I agree with you, lowerCamelCase looks great. I was going to mention flowchart-elk but since you made it an option then it has been standardized.

I'm not certain how we should handle those. Especially elk. For stateDiagram-v2, we can simply use state, as any new diagrams using the state keyword will be v2, and older diagrams with stateDiagram-v2 will continue working as the changes MUST be backwards compatible.

But flowchart ELK, is simply a different layout engine, so maybe we could support elk as an optional keyword like we support orientation.

flow [elk], so both flow elk and flow will be supported.

I'm not much familiar with C4 diagrams, but c4 context|container|component... seems like a suitable option.

Yeah, it seems like this is a good approach.

@Yokozuna59
Copy link
Member

So to summarize what we need to change:

  • Update the .jison grammar to support what have been stated above without breaking existing grammar.
  • Add a tip tilling users to use new keyword instead of old ones.
  • update all docs to make them use the nee keywords so that users don't use old ones.
  • modify most use cases to use the new keyword (at least there is one test case to make sure the old one is working fine).
  • If the the keyword has been changed into an option, it must be illustrated more in docs.

What else? I'll work update some while I have time.


Add by adding these changes, will it affect the structure stated in #4499? Do you have something in that? Even if our current diagrams didn't faced this issue, I think we should discuss this case since it might occur with xychart and linebar.

I'm aware that the root message is kind of outdated, but I haven't been receiving any feedback on the suggested ideas. Could I assume that they have been approved with those likes? @sidharthv96

@Yokozuna59
Copy link
Member

flowchart-v2 and sankey are messing.


I'm not familiar with those -v2 diagrams; is it a grammar or layout difference?

@grimley517
Copy link

Two observations.

Firstly I'm not sure about "er" for Entity Relationship. It seems that is being overly abbreviated to the point of making its purpose obscure to new users, or users whose American English fluency is low. I'd proffer the suggestion of "Entity".

Secondly, and this is probably more from my poor understanding of open source. Is there a preferred or defined obsolescence process? Or is the scope for this issue just the addition of new terms?

@nirname
Copy link
Contributor

nirname commented Apr 26, 2024

@grimley517 I think issue is about right. Or if you have any other suggestions on renaming keywords then it probably can be referred via "syntax change" issue. There is no exact process on eliminating legacy code / syntax / options, so you feel that none of the existing issues fit your ideas open a new one. You may also start a discussion if it is kinda sketchy and later or we can create an issue based on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants