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 backslash bugs in convert feature #1742

Merged
merged 10 commits into from
Feb 2, 2022
Merged

Conversation

ong6
Copy link
Contributor

@ong6 ong6 commented Jan 30, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Fixes #1736

Overview of changes:

Updated the relativePagePath to ensure that no unnecessary \ were being made. Done by updating a function from fsUtil

Anything you'd like to highlight / discuss:

Testing instructions:

Run markbind init -convert on a test repo and make sure there are no instances of \ or %5C in the layouts/default.md file.
Build and see that there also should not have any instance of \ or %5C

Proposed commit message: (wrap lines at 72 characters)

Resolved bug regarding backslashes in convert feature


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features (will be done in another PR)
  • Linked all related issues
  • No unrelated changes

@ong6 ong6 changed the title Backslash fix Fix backslash bugs in convert feature Jan 30, 2022
Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Saw that in one of the commits you updated the package-lock.json. Is there a reason for that?

Otherwise, LGTM 👍

@ong6
Copy link
Contributor Author

ong6 commented Jan 31, 2022

Saw that in one of the commits you updated the package-lock.json. Is there a reason for that?

Otherwise, LGTM 👍

I thought my package-lock.json was failing the Appveyor build. But then realized it was another issue altogether. Also, I think zeyu asked me to remove some lines from the PR #1727.

@jonahtanjz
Copy link
Contributor

Thanks for clarifying @ong6 :)

@ang-zeyu
Copy link
Contributor

I thought my package-lock.json was failing the Appveyor build. But then realized it was another issue altogether. Also, I think zeyu asked me to remove some lines from the PR #1727.

#1727 (comment) Sorry for the ambiguous wording.

To clarify, I meant removing the diffs. (same rationale as #1679 (comment))

fyi, the issue with appveyor: https://github.com/actions/setup-node/issues/411
I think we can make a mental note of testing the windows build locally for now and ignore the failing builds @jonahtanjz @ryoarmanda

@ang-zeyu
Copy link
Contributor

I've reverted the package-lock changes from #1728, which seems to be causing the changes here.
You can remove the diffs by merging in the latest changes too :)

@ong6 ong6 closed this Jan 31, 2022
@ong6 ong6 deleted the backslash-fix branch January 31, 2022 12:24
@ong6 ong6 restored the backslash-fix branch January 31, 2022 15:11
@ong6 ong6 reopened this Jan 31, 2022
@jonahtanjz jonahtanjz merged commit 580c045 into MarkBind:master Feb 2, 2022
@jonahtanjz jonahtanjz added this to the 4.0 milestone Feb 2, 2022
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.

While using markbind convert, some / end up as \
3 participants