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

Minor bugfixes #1

Merged
merged 29 commits into from
Feb 11, 2019
Merged

Minor bugfixes #1

merged 29 commits into from
Feb 11, 2019

Conversation

karimatthews
Copy link
Owner

@karimatthews karimatthews commented Jan 25, 2019

Description

This PR aims to fix some minor bugs in the Jira to markdown library.

The primary issue addressed in this PR (and the reason for creating this fork) is the issue of intra-word formatting when Jira-flavoured markdown is converted into regular markdown.

Before Fix
phrase_with_underscores in Jira markdown is transformed to phrase*with*underscores in regular markdown. This would then be rendered so:

image

This is not desirable behaviour, particularly for important content such as urls and emails.

In the other direction, phrase*with*asterisks in regular markdown becomes phrase_with_asterisks in Jira markdown, which is rendered so:

image

This is also undesirable.

After fix
This library should recognise the * and _ characters within words should not be transformed.

Other Changes:

  • Fixes for broken tests
  • Added dependencies
  • Small fix for maintaining whitespaces either side of strikethroughs
  • Support conversion of multiple codeblocks
  • Prevent formatting being applied within codeblocks and noformat blocks

Note

There are two skipped test. My markdown expertise has so far proved insufficient for getting this case to pass. (When a snippet like a**phrase** is converted from md to jira it will be formatted to a*phrase*.) I think this is acceptable bug to leave for now for two reasons:

  1. I can't think of a use case and
  2. This is the current behaviour rather than a new bug

If someone can think of a fix or has a use case that suggests this really needs to be fixed please let me know.

References

@karimatthews karimatthews self-assigned this Jan 25, 2019
Copy link

@taonic taonic left a comment

Choose a reason for hiding this comment

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

LGTM 👍

index.js Outdated Show resolved Hide resolved
@karimatthews karimatthews changed the title [WIP] Minor bugfixes Minor bugfixes Feb 1, 2019
index.js Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
});

// TODO: Fix the code so this test can be unskipped
it.skip('does not apply bold formatting without an asterisk pair at the start of the phrase', function () {
Copy link

Choose a reason for hiding this comment

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

As this PR fixes minor issue, why do you skip this test - was it existing issue?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, it was an existing issue. See the Note in the PR description.

package.json Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
test/jira2md.js Outdated Show resolved Hide resolved
test/jira2md.js Show resolved Hide resolved
index.js Show resolved Hide resolved
@veera
Copy link

veera commented Feb 11, 2019

@karimatthews 👍

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.

5 participants