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 crash in warning message generation and bug in reporting #144

Merged
merged 3 commits into from
May 19, 2017

Conversation

KronicDeth
Copy link
Contributor

I can demonstrate the below bugs on C-S-D/retort@8afabf4bb2e04ef83a27a820fff772394cee3883 if you need something to test this against for verification.

Changelog

Bug Fixes

  • add_message calls Agent.update, which always returns :ok, so it doesn't make sense to use the return value for the new options1 in _parse. Since both paths shouldn't alter options, have the case only be run for side-effect of add_message call.

    Fixes this error

    16:21:23.570 [error] Task #PID<0.512.0> started from #PID<0.70.0> terminating
    ** (BadMapError) expected a map, got: :ok
        (earmark) lib/earmark/block.ex:177: Earmark.Block._parse/3
        (earmark) lib/earmark/block.ex:65: Earmark.Block.lines_to_blocks/2
        (earmark) lib/earmark/block.ex:56: Earmark.Block.parse/2
        (earmark) lib/earmark.ex:337: Earmark.parse/2
        (earmark) lib/earmark.ex:321: Earmark._as_html/2
        (earmark) lib/earmark.ex:314: Earmark.as_html!/2
        (ex_doc) lib/ex_doc/markdown.ex:32: ExDoc.Markdown.to_html/2
        (ex_doc) lib/ex_doc/formatter/html.ex:192:
    ExDoc.Formatter.HTML.build_extra/6
    Function: #Function<14.119834956/0 in
    ExDoc.Formatter.HTML.build_extras/3>
        Args: []
    
  • The Agent's state in Earmark.Global.Messages is never reset, so once a line has a warning it has that warning for all subsequent files. Since get_all_messages() is never called twice to report the same warnings intentionally, get_all_messages can be changed to pop_all_messages, so it gets the messages as before, but also resets the state, in preparation for the next call.

    This fixes errors like

    CHANGELOG.md:41: warning: Closing unclosed backquotes ` at end of input
    lib/retort/meta.ex:41: warning: Closing unclosed backquotes ` at end of input
    lib/retort.ex:41: warning: Closing unclosed backquotes ` at end of input
    

    where the only the first warning is real, but it is reported on all
    following files.

Incompatible Changes

  • Earmark.Global.Messages.get_all_messages/0 is replaced by Earmark.Global.Messages.pop_all_messages/0 to prevent leaking messages between files.

KronicDeth and others added 3 commits May 18, 2017 16:21
add_message calls Agent.update, which always returns `:ok`, so it
doesn't make sense to use the return value for the new `options1`.
Since both paths shouldn't alter `options`, have the `case` only be run
for side-effect of add_message call.

Fixes this error

```
16:21:23.570 [error] Task #PID<0.512.0> started from #PID<0.70.0>
terminating
** (BadMapError) expected a map, got: :ok
    (earmark) lib/earmark/block.ex:177: Earmark.Block._parse/3
    (earmark) lib/earmark/block.ex:65: Earmark.Block.lines_to_blocks/2
    (earmark) lib/earmark/block.ex:56: Earmark.Block.parse/2
    (earmark) lib/earmark.ex:337: Earmark.parse/2
    (earmark) lib/earmark.ex:321: Earmark._as_html/2
    (earmark) lib/earmark.ex:314: Earmark.as_html!/2
    (ex_doc) lib/ex_doc/markdown.ex:32: ExDoc.Markdown.to_html/2
    (ex_doc) lib/ex_doc/formatter/html.ex:192:
ExDoc.Formatter.HTML.build_extra/6
Function: #Function<14.119834956/0 in
ExDoc.Formatter.HTML.build_extras/3>
    Args: []
** (EXIT from #PID<0.70.0>) an exception was raised:
    ** (BadMapError) expected a map, got: :ok
        (earmark) lib/earmark/block.ex:177: Earmark.Block._parse/3
        (earmark) lib/earmark/block.ex:65:
Earmark.Block.lines_to_blocks/2
        (earmark) lib/earmark/block.ex:56: Earmark.Block.parse/2
        (earmark) lib/earmark.ex:337: Earmark.parse/2
        (earmark) lib/earmark.ex:321: Earmark._as_html/2
        (earmark) lib/earmark.ex:314: Earmark.as_html!/2
        (ex_doc) lib/ex_doc/markdown.ex:32: ExDoc.Markdown.to_html/2
        (ex_doc) lib/ex_doc/formatter/html.ex:192:
ExDoc.Formatter.HTML.build_extra/6
```
The Agent's state is never reset, so once a line has a warning it has
that warning for all subsequent files.  Since `get_all_messages()` is
never called twice to report the same warnings intentionally,
`get_all_messages` can be changed to `pop_all_messages`, so it gets the
messages as before, but also resets the state, in preparation for the
next call.

This fixes errors like

```
CHANGELOG.md:41: warning: Closing unclosed backquotes ` at end of input
lib/retort/meta.ex:41: warning: Closing unclosed backquotes ` at end of
input
lib/retort.ex:41: warning: Closing unclosed backquotes ` at end of input
```

where the only the first warning is real, but it is reported on all
following files.
@RobertDober
Copy link
Collaborator

Many thanks for this one, I had designed the lifetime for the call of Earmark.as_html* but this is a nice improvement.

@RobertDober RobertDober merged commit 103af39 into pragdave:master May 19, 2017
@KronicDeth KronicDeth deleted the badmaperror branch May 19, 2017 12:24
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.

2 participants