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 extra newline in convert feature #1746

Merged
merged 15 commits into from
Feb 2, 2022
Merged

Conversation

ong6
Copy link
Contributor

@ong6 ong6 commented Jan 31, 2022

What is the purpose of this pull request?

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

Fixes #1726
Closes #1727

Overview of changes:

This was caused by the extra newline created at the end of buildSiteNav() in the core/site dir. I simply added a trimEnd() to fix the issue.

Anything you'd like to highlight / discuss:

Testing instructions:

Run markbind init -convert on a test repo and see that there is no more new line before the closing tag.
When running tests, the Home.html in test_navigation_convert should not contain any unnecessary spaces in the navigation component.

Proposed commit message: (wrap lines at 72 characters)

Fixed convert command generating extra newline


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@ong6 ong6 changed the title Newline fix Fix extra newline in convert feature Jan 31, 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.

One suggestion, otherwise LGTM!

@@ -436,14 +436,14 @@ class Site {
.filter(addressablePage => !addressablePage.src.startsWith('_'))
.forEach((page) => {
const addressablePagePath = path.join(this.rootPath, page.src);
const relativePagePathWithoutExt = fsUtil.removeExtension(
const relativePagePathWithoutExt = fsUtil.removeExtensionPosix(
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to rebase to master so this PR doesn't overwrite the changes that has already been merged.

@ang-zeyu ang-zeyu added this to the 4.0 milestone Feb 2, 2022
@ang-zeyu ang-zeyu merged commit ea61d81 into MarkBind:master Feb 2, 2022
@ang-zeyu ang-zeyu mentioned this pull request Feb 9, 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.

When using the convert feature, an extra newline is generated
3 participants