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

Bug fixes: Handling of link markdown #6922

Merged
merged 12 commits into from
Feb 11, 2022
Merged

Conversation

janusbal
Copy link
Contributor

@janusbal janusbal commented Feb 8, 2022

Description

Fixes for:

  1. Link markdown format: When a link destination has parenthesis the formatting is broken. E.g:
    link renders as link with destination of https://adaptivecard.io/(aaa

How Verified

To test for markdown:
{ "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", "type": "AdaptiveCard", "version": "1.0", "body": [ { "type": "TextBlock", "text": "[empty destination]()", "wrap": true }, { "type": "TextBlock", "text": "[destination with [a](x)]([https://aaa.com](https://bbb.com))", "wrap": true }, { "type": "TextBlock", "text": "[link]([https://aaa][.c[]om](https://bbb.com))", "wrap": true }, { "type": "TextBlock", "text": "[link()[][]]([https://aaa][.c[]om](*ht*tp([])s://bbb.com))", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know [aaa] m (aa)ore](https://aaa.bbb.com/(a)sites(Test).doc?somegar)bageValue*afterstar()", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](https://aaa.bbb.com/(a)sites(Test).doc?somegarbageValue)*afterstar()", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](https://aaa.bbb.com/(a)sit(es(Test).doc?somegarbageValue)*after_beforestar)*", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](https://aaa.bbb.com/(a)sites(Test).doc?somegarbageValue)*afterstar()", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](google.com)", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](https://aaa.bbb.com/(a)sit(es(Test).doc?somegarbageValue)*afterstar)", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more]()", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know)]()betweenpars()", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more](", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more])", "wrap": true }, { "type": "TextBlock", "text": "[Stay tuned to Know more]", "wrap": true } ] }

NOTE: This change still does not support filtered markdowns. For example:
[atitle with *italic*](http://aa.com) will not render as other systems would. atitle with italic

[empty destination]()
[destination with [a](x)]([https://aaa.com](https://bbb.com))
[link]([https://aaa][.c[]om](https://bbb.com))
[link()[][]]([https://aaa][.c[]om](*ht*tp([])s://bbb.com))
[Stay tuned to Know [aaa] m (aa)ore](https://aaa.bbb.com/(a)sites(Test).doc?somegar)bageValue*afafafafafafa()
[Stay tuned to Know more](https://aaa.bbb.com/(a)sites(Test).doc?somegarbageValue)*afafafafafafa()
[Stay tuned to Know more](https://aaa.bbb.com/(a)sit(es(Test).doc?somegarbageValue)*afafafafafafa)*
[Stay tuned to Know more](https://aaa.bbb.com/(a)sites(Test).doc?somegarbageValue)*afafafafafafa()
[Stay tuned to Know more](google.com)
[Stay tuned to Know more](https://aaa.bbb.com/(a)sit(es(Test).doc?somegarbageValue)*afafafafafafa)
[Stay tuned to Know more]()
[Stay tuned to Know)]()afafafa()
[Stay tuned to Know more](
[Stay tuned to Know more])
[Stay tuned to Know more]"

image

Microsoft Reviewers: Open in CodeFlow

@janusbal janusbal changed the title Bug fixes: iOS Autolayout | Handling of link markdown | Parsing warnings Bug fixes: Textblock layout | Handling of link markdown | Parsing warnings Feb 8, 2022
Copy link
Member

@jwoo-msft jwoo-msft left a comment

Choose a reason for hiding this comment

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

@janusbal
Can you separate this PR into two PRs?
I can approve two issues, but markdwon takes longer for me to parse.In that way, markdown review won't block the rest of the prs.

@janusbal
Copy link
Contributor Author

janusbal commented Feb 8, 2022

@janusbal Can you separate this PR into two PRs? I can approve two issues, but markdwon takes longer for me to parse.In that way, markdown review won't block the rest of the prs.

@jwoo-msft we prefer to have the next sdk drop as many fixes as we can possibly include. We're fine with markdown changes going to more reviewers.

@janusbal janusbal closed this Feb 8, 2022
@ghost ghost removed the Needs: Author Feedback label Feb 8, 2022
@janusbal janusbal reopened this Feb 8, 2022
@janusbal janusbal changed the title Bug fixes: Textblock layout | Handling of link markdown | Parsing warnings Bug fixes: = Handling of link markdown Feb 9, 2022
@janusbal janusbal changed the title Bug fixes: = Handling of link markdown Bug fixes: Handling of link markdown Feb 10, 2022
Copy link
Member

@jwoo-msft jwoo-msft left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.
It looks great overall with few minor issues.

source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.h Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
source/shared/cpp/ObjectModel/MarkDownBlockParser.cpp Outdated Show resolved Hide resolved
@ghost ghost removed the Needs: Author Feedback label Feb 10, 2022
@jwoo-msft jwoo-msft merged commit 9442e17 into microsoft:main Feb 11, 2022
jwoo-msft pushed a commit that referenced this pull request Feb 16, 2022
* need to take accoutn for empty

* works but need to find memory leak

* testing against stackedit

* removing unneeded line | allocating array

* clean up

* getting rid of str() use from strigstream

* handling link destination with [ character and other tokens

* removing the ios changes

* resolving comments

* resolving comments

* clan format

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>
@jwoo-msft jwoo-msft mentioned this pull request Feb 16, 2022
jwoo-msft added a commit that referenced this pull request Feb 17, 2022
* Added App extension to podspec (#6849)

* app extension model added

updated spec

added podspec at root for testing\n revert it

spec updated

updated pod spec

pod spec update

moretesting

[updated macro name]

* updated podspec

* [ios] Updated to final podspec version

* update compiler flag

* pod spec update

* updated flag name

* [iOS] fixed empty filtered list issue (#6528)

* [iOS] fixed empty filtered list issue

* [iOS] changed to function call

Co-authored-by: Canhua Li <licanhua@live.com>

* moving resource bundle to core (#6853)

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>

* Rounding off to 2 decimals (#6863)

* [ios] updated round off logic (#6866)

* [ios] updated round off logic

* [ios] refactored the code

* [ios] fixed project file

* [ios] fixed log

* [ios] spec updated

* update test (#6864)

* added heartbeat test ios build infrastructure (#6842)

* [iOS] added yml version of ci

updated path

[ios] path update

[iOS] variable updated

[ios] added new test scheme

* [iOS] added spec linting task

added missing test file

updated spect lint task

added error handling

added parallelism to jobs

* added cert test

added cert install yml

updated pass

Update ios-heartbeat.yml for Azure Pipelines

added provisional file

added script

unimportant commit

unimportant commit

unimportant commit

* added e-mail message

simple update

added e-mail message

added e-mail message

added e-mail message

updated e-mail message

updated e-mail message

updated e-mail message

* updated cron job

* added false test

Revert "added false test"

This reverts commit ef302f0.

* added false test

added build task to cert task

added build task to cert task

added build task to cert task

good cert bad prov

added app push

* parameterized template

updated parameters

updated params

updated parameters

updated to correct provisional file

* updated trigger

* updated e-mail address

* Bug fixes: Textblock layout | Parsing warnings (#7041)

* need to take accoutn for empty

* works but need to find memory leak

* testing against stackedit

* removing unneeded line | allocating array

* clean up

* getting rid of str() use from strigstream

* handling link destination with [ character and other tokens

* removing cpp changes

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>

* Bug fixes: Handling of link markdown  (#6922)

* need to take accoutn for empty

* works but need to find memory leak

* testing against stackedit

* removing unneeded line | allocating array

* clean up

* getting rid of str() use from strigstream

* handling link destination with [ character and other tokens

* removing the ios changes

* resolving comments

* resolving comments

* clan format

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>

* updated android-ci yml to use latest macOs

Co-authored-by: Canhua Li <licanhua@live.com>
Co-authored-by: janusbal <86079148+janusbal@users.noreply.github.com>
Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>
Co-authored-by: shabripabba <87400370+shabripabba@users.noreply.github.com>
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
* need to take accoutn for empty

* works but need to find memory leak

* testing against stackedit

* removing unneeded line | allocating array

* clean up

* getting rid of str() use from strigstream

* handling link destination with [ character and other tokens

* removing the ios changes

* resolving comments

* resolving comments

* clan format

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>
rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
* need to take accoutn for empty

* works but need to find memory leak

* testing against stackedit

* removing unneeded line | allocating array

* clean up

* getting rid of str() use from strigstream

* handling link destination with [ character and other tokens

* removing the ios changes

* resolving comments

* resolving comments

* clan format

Co-authored-by: Janus Balatbat <jsbalatbat@microsoft.com>
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.

None yet

3 participants