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

Diagram Template #4696

Closed
wants to merge 1 commit into from
Closed

Diagram Template #4696

wants to merge 1 commit into from

Conversation

nirname
Copy link
Contributor

@nirname nirname commented Aug 3, 2023

📑 Disclaimer! This is only a proposition, not a PR

Simply it makes much easier to see the structure of proposed changes in there, instead of discussing them in the issue.

Originally there was a PR, #4615, but I decided to move this branch to the main repository instead of fork, so that we all could commit here and use it as a reference.

It is initial commit of another bransh, a snapshot of the development at some point where it is relative easy to see what it takes to create a new diagram.

May be it can be implemented as a separate package.

Related #4606

It started as a railroad diagram, but obviously all the railroad names must be later on cut from here and replaced by a parameter

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

📋 Tasks

Make sure you

@github-actions github-actions bot added the Type: Enhancement New feature or request label Aug 3, 2023
@nirname nirname force-pushed the feature/4606-diagram-template branch from 1e5fb79 to 6a89e28 Compare August 3, 2023 14:28
@nirname
Copy link
Contributor Author

nirname commented Aug 3, 2023

Rebased it on the fresh development branch

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #4696 (6a89e28) into develop (b87f1f2) will increase coverage by 1.20%.
The diff coverage is 71.42%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4696      +/-   ##
===========================================
+ Coverage    77.00%   78.21%   +1.20%     
===========================================
  Files          144      144              
  Lines        14583    10737    -3846     
  Branches       563      559       -4     
===========================================
- Hits         11230     8398    -2832     
+ Misses        3243     2214    -1029     
- Partials       110      125      +15     
Flag Coverage Δ
e2e 78.17% <71.42%> (-5.72%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...s/mermaid/src/diagram-api/diagram-orchestration.ts 83.33% <ø> (+43.79%) ⬆️
.../mermaid/src/diagrams/railroad/railroadDetector.ts 71.42% <71.42%> (ø)

... and 71 files with indirect coverage changes

@nirname nirname mentioned this pull request Aug 3, 2023
4 tasks
@nirname
Copy link
Contributor Author

nirname commented Aug 3, 2023

@Yokozuna59 you can commit here directly

Comment on lines +6 to +9
// import { prepareTextForParsing } from './railroadUtils.js';

// const originalParse = parser.parse.bind(parser);
// parser.parse = (text: string) => originalParse(prepareTextForParsing(text));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this lines here intentionally. I don't know how to implement this in a beautiful way, but there is a need to pre-process diagram code before parsing. Even if we dont need it in the majority of the cases we have to provide developers with a possibility to do it in a standard way before parsing, otherwise everyone will do it his own way

@nirname
Copy link
Contributor Author

nirname commented Aug 24, 2023

I think even closed request is OK as a reference, so as not to pollute our PR's queue, so I am closing it

@nirname nirname closed this Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant