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

Support `atom` protocol links when links are handled #16940

Merged
merged 3 commits into from Sep 6, 2018

Conversation

Projects
None yet
3 participants
@BoykoAlex
Copy link
Contributor

BoykoAlex commented Mar 13, 2018

Requirements

Should fix #16888 Markdown preview and hover/datatips MD links with atom:// URLs should have the right handling.

Description of the Change

This seems to be the right spot to fix it, however there might be a more elegant solution invoking the URI handler since urlHandlerRegistry seem to be meant to be a private member rather than public. Nevertheless, it makes the use case I had trouble with work nicely.

Why Should This Be In Core?

atom:// registry and handling points belongs to the core

Benefits

Ability to put source links in hover documentation

Possible Drawbacks

Not sure

Verification Process

Ad-hoc manual testing of MD preview and hovers containing valid atom:// links

Applicable Issues

#16888

@50Wliu 50Wliu referenced this pull request May 1, 2018

Merged

add atom:// hyperlinks #21

@UziTech UziTech referenced this pull request May 1, 2018

Merged

add atom: protocol #33

@BoykoAlex

This comment has been minimized.

Copy link
Contributor Author

BoykoAlex commented May 14, 2018

Is there stuff to fix for me? Please let me know :-)

@daviwil

This comment has been minimized.

Copy link
Member

daviwil commented May 16, 2018

Hey @BoykoAlex, sorry for the delay. I've been seeing some weird CI failures during the snapshot phase that I can't explain so I'm going to restart it again to see if it's resolved. If it's still broken we may need to investigate further!

@UziTech

This comment has been minimized.

Copy link
Contributor

UziTech commented Aug 27, 2018

Any update on this?

@asheren asheren referenced this pull request Sep 5, 2018

Closed

Iteration Plan: September 4 - September 14 #17984

0 of 17 tasks complete
@daviwil

This comment has been minimized.

Copy link
Member

daviwil commented Sep 6, 2018

Finally got a clean VSTS build, merging this now. Thanks @BoykoAlex!

@daviwil daviwil merged commit c03ddeb into atom:master Sep 6, 2018

2 of 3 checks passed

Atom Pull Requests #6299 failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.