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

Fix preview opening relative link #400

Merged
merged 7 commits into from
Apr 11, 2021
Merged

Conversation

rongcuid
Copy link
Contributor

Fixes #397. I haven't tested very comprehensively, but I tried a few common cases (that I encounter).

@rongcuid
Copy link
Contributor Author

rongcuid commented Apr 5, 2021

Is there anything I can do to help with the preview process?

Copy link
Contributor

@danyill danyill left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution and your patience with the review process.

This PR doesn't currently pass an npm lint but the fix is easy. If we could remove extraneous commented code that would be good (this repo already suffers from an overdose of that) and I think the additional task is not required.

If you agree could you please make the modifications.

src/features/preview.ts Outdated Show resolved Hide resolved
src/features/preview.ts Outdated Show resolved Hide resolved
src/features/preview.ts Outdated Show resolved Hide resolved
src/text-parser.ts Outdated Show resolved Hide resolved
src/text-parser.ts Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
@rongcuid
Copy link
Contributor Author

All should be resolved

@joaompinto
Copy link
Contributor

LGTM

Copy link
Contributor Author

@rongcuid rongcuid left a comment

Choose a reason for hiding this comment

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

I didn't set up linting myself (not familiar with node.js), but I think that is all changes.

@rongcuid
Copy link
Contributor Author

Is there something I still need to do? It still says Changes requested. Not sure if I need to confirm something somewhere.

@danyill
Copy link
Contributor

danyill commented Apr 11, 2021

LGTM, thanks @rongcuid for these changes and for this improvement.

@joaompinto if you have the time could we have a release pelase? (Incidentally I plan to to do some work on this extension and winter is soon approaching which will provide more time -- there are several other deprecations which will probably bite soon).

@danyill danyill merged commit dcd2ffd into asciidoctor:master Apr 11, 2021
@joaompinto
Copy link
Contributor

@danyill I have been a bit inactive lately, the release to marketplace is automated, you just need to create a new tag and push it, do you want to try it ?

@danyill
Copy link
Contributor

danyill commented Apr 11, 2021

@danyill I have been a bit inactive lately, the release to marketplace is automated, you just need to create a new tag and push it, do you want to try it ?

A little scary but yes that worked beautifully, thanks 😄

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.

Unable to open relative links
3 participants