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

Use MfmUrl AST in MfmLink AST #126

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

shibaobun
Copy link

What

  • Use MfmUrl AST in MfmLink AST
  • Add test cases for reproducting bracket urls within links

Why

Additional info (optional)

This is my first time using typescript, so please let me know if there's anything that can be changed for best practice!
Thank you!

@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Merging #126 (88daccc) into develop (ae92c2a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 88daccc differs from pull request most recent head e7f70c6. Consider uploading reports for the commit e7f70c6 to get more accurate results

@@           Coverage Diff            @@
##           develop     #126   +/-   ##
========================================
  Coverage    97.72%   97.73%           
========================================
  Files            8        8           
  Lines         1541     1544    +3     
  Branches       317      233   -84     
========================================
+ Hits          1506     1509    +3     
- Misses          33       34    +1     
+ Partials         2        1    -1     
Impacted Files Coverage Δ
src/internal/parser.ts 99.33% <100.00%> (ø)
src/internal/util.ts 97.63% <100.00%> (ø)
src/node.ts 100.00% <100.00%> (ø)
src/api.ts 100.00% <0.00%> (ø)
src/internal/index.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shibaobun
Copy link
Author

@syuilo i've rebased this PR with your changes, please let me know if there's anything you'd like changed!

Copy link
Member

@anatawa12 anatawa12 left a comment

Choose a reason for hiding this comment

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

破壊的変更になるし MfmLink の props に brackets フラグを追加するのほうがいい気がするのですがsyuiloさんはどう考えますか


追記: 余計なdiffが大量に入ってるのは私は良くないと考えてたりします

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.

<>付きのURLのリンクをparseしてtoStringすると<>の情報が消える
2 participants