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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: field static classifier #2269

Merged
merged 7 commits into from
Sep 13, 2021
Merged

Feature: field static classifier #2269

merged 7 commits into from
Sep 13, 2021

Conversation

Xstoudi
Copy link
Contributor

@Xstoudi Xstoudi commented Aug 26, 2021

馃搼 Summary

The PR allow people to add static field classifier on class fields.

Resolves #1953

馃搹 Design Decisions

No design decisions where needed, I just made it the way it was already made for methods.

馃搵 Tasks

Make sure you

  • 馃摉 have read the contribution guidelines
  • 馃捇 have added unit/e2e tests (if appropriate)
  • 馃敄 targeted develop branch

@Xstoudi Xstoudi changed the title Feature field classifier Feature: field static classifier Aug 26, 2021
@ashishjain0512
Copy link
Collaborator

ashishjain0512 commented Aug 26, 2021

Hey, really good work. Much appreciated. We are now moving away from svgDraw (based on d3) to a newer rendering engine based on dagre-wrapper.
Can you add similar changes in the class_box (src/dagre-wrapper/nodes.js). as in the next release, we will shift class diagrams to this rendering engine.
Once this is done, we will be happy to merge this PR.

@Xstoudi
Copy link
Contributor Author

Xstoudi commented Aug 27, 2021

It's done ! Let me know if something is wrongly done.

@Xstoudi
Copy link
Contributor Author

Xstoudi commented Aug 27, 2021

Just to be sure, are you aware that dagre seems to indicate it's deprecated?

@knsv knsv merged commit e01ad57 into mermaid-js:develop Sep 13, 2021
@Dampfwalze
Copy link

For some reason, this currently only works in specific cases:

classDiagram
  class Person {
  String name$
  name : String$
  name$ : String
  name$
  }
  Person: int age$
  Person: age$
classDiagram
  class Person {
    String name$
    name : String$
    name$ : String
    name$
  }
  Person: int age$
  Person: age$

Why doesn't it just check, if the line is a field and ends with a $ or *? It currently seems to do some shenanigans, that only allows it to work if the field is formatted in a specific way:
First Character may be one of + # - ~, one word without any special character, a whitespace, one word without any special character, ends with $ or *
As RegEx something like this: (\+|-|~|#)?[a-zA-Z0-9] [a-zA-Z0-9](\$|\*)

It should be more like this: .*(\$|\*)

skogsbaer added a commit to skogsbaer/mermaid that referenced this pull request Nov 26, 2022
Previously, the static classifier was only supported for attributes
using the syntax "String name$". Now you can also write "name: String$"
See PR mermaid-js#2269 and issue mermaid-js#3001 and my old issue mermaid-js#1953

Further, it was confusing that static/abstract classifiers for methods
had to be written after the closing parenthesis of the method signature,
even if the method has a return type. So you had to write
"foo(name: String)* int".

This commit also adds the possibility of writing the classifier at the
very end, as in "foo(name: String) int*".
@skogsbaer
Copy link

@Dampfwalze There is PR #3855 which addresses your issues.

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

Successfully merging this pull request may close these issues.

Static instance variables for class diagram
5 participants