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

Add compact_output option #394

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

rinpatch
Copy link
Contributor

As discussed in
#393 (comment).
I did not name the option "single_line", since newlines in code blocks
are preserved even with it on and since Earmark passes
the single newlines from the input to the final output.

Also removes some string concatenations and replaces single-character
binaries with character codes.

I did not find a file the tests for this feature would fit in, so
I created a new one at test/acceptance/html/compact_output_test.exs

Closes #391

As discussed in
pragdave#393 (comment).
I did not name the option "single_line", since newlines in code blocks
are preserved even with it on and since Earmark passes
the single newlines from the input to the final output.

Also removes some string concatenations and replaces single-character
binaries with character codes.

I did not find a file the tests for this feature would fit in, so
I created a new one at `test/acceptance/html/compact_output_test.exs`

Closes pragdave#391
@RobertDober RobertDober merged commit 062a68a into pragdave:master Nov 26, 2020
@RobertDober
Copy link
Collaborator

Ty for your work, appreciated I will plan a release within the week

@RobertDober
Copy link
Collaborator

@rinpatch scratch the last remark, I have 10 failing tests after the merge (but only locally Travis is still happy) I have to check that out first

@RobertDober
Copy link
Collaborator

So here is the problem I am having

Interactive Elixir (1.11.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Earmark.as_html("[link](/uri \"title\") [link](/uri \"title\")")
{:ok,
 "<p>\n<a href=\"/uri\" title=\"title\">link</a><a href=\"/uri\" title=\"title\">link</a></p>\n",
 []}

There is not a space anymore between the two links which is clearly needed, hopefully I can fix this but I will do so in a side branch and revert the merge, sorry!

BTW. Still no idea why Travis does not catch these errors, you should see them locally too

here is the corresponding test

mix test test/acceptance/html/links_images/titles_test.exs:5                                                                                                     [14:33:07]
Excluding tags: [:test]
Including tags: [line: "5"]



  1) test Links with titles two titled links (Acceptance.Html.LinksImages.TitlesTest)
     test/acceptance/html/links_images/titles_test.exs:5
     Assertion with == failed
     code:  assert as_html(markdown) == {:ok, html, messages}
     left:  {:ok, "<p>\n<a href=\"/uri\" title=\"title\">link</a><a href=\"/uri\" title=\"title\">link</a></p>\n", []}
     right: {:ok, "<p>\n<a href=\"/uri\" title=\"title\">link</a> <a href=\"/uri\" title=\"title\">link</a></p>\n", []}
     stacktrace:
       test/acceptance/html/links_images/titles_test.exs:11: (test)



Finished in 0.08 seconds
2 tests, 1 failure, 1 excluded

Randomized with seed 667380
```

@RobertDober
Copy link
Collaborator

@rinpatch your patch is fine this is just a strange hiccup I have to sort out sorry for the delay, please stand by.

@RobertDober
Copy link
Collaborator

@rinpatch I managed to clean up the mess on my local machines, and just released v1.4.12 which, of course, contains your PR again

Well done and thank you

@tmjoen
Copy link

tmjoen commented Dec 3, 2020

This is great! I have been on 1.4.4 for a while now due to the whitespace changes, but this passed all my tests except one:

iex(1)> Earmark.as_html!("<div class=\"lead\">**Some** text here.</div>")
"<div class=\"lead\">\n  **Some** text here.</div>\n"
iex(2)> Earmark.as_html!("<div class=\"lead\">**Some** text here.</div>", compact_output: true)
"<div class=\"lead\">  **Some** text here.</div>"

There seems to be 2 extra spaces inserted in nr 2 there :)

@RobertDober
Copy link
Collaborator

RobertDober commented Dec 3, 2020

@tmjoen can you please open an issue too late, I have already fixed it

@RobertDober
Copy link
Collaborator

... and released 1.4.13

@tmjoen
Copy link

tmjoen commented Dec 3, 2020

Haha, thanks @RobertDober ! You're too fast for me 😅

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.

Option to minify HTML output
3 participants