MarkdownBear: Add `validate-links` plugin #1919

yash-nisar · 2017-07-14T07:10:10Z

Closes #924

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

yash-nisar · 2017-07-14T07:10:24Z

CC @Makman2

sils · 2017-07-15T15:27:51Z

bears/markdown/MarkdownBear.py


+        args += ['--use', 'validate-links']


shouldn't we have an option for this?

orientate on our InvalidLinksBear regarding setting names 👍

@Makman2 Our InvalidLinkBear has no such setting. I've also checked the UrlBear on which the former bear depends. 😉

Yeah if you enable InvalidLinkBear you definitely want to check invalid links :) In this case this should be optional 👍 Internet accesses aren't always desired by people that want to lint their code 👍

No No, I meant I've checked the InvalidLinkBear for a similar setting name. Is the PR good ?

Makman2 · 2017-07-15T15:37:15Z

bears/markdown/MarkdownBear.py

    AUTHORS = {'The coala developers'}
    AUTHORS_EMAILS = {'coala-devel@googlegroups.com'}
    LICENSE = 'AGPL-3.0'
    CAN_FIX = {'Formatting'}

    _output_regex = re.compile(
        r'\s*(?P<line>\d+):(?P<column>\d+)'
-        r'\s*(?P<severity>warning)\s*(?P<message>.*)')
+        r'\s*(?P<severity>warning)\s*(?P<message>.+?)(?:  .*|\n|$)')


is this maybe a separate commit?

Makman2 · 2017-07-15T16:52:46Z

bears/markdown/MarkdownBear.py

@@ -127,22 +128,23 @@ def create_arguments(filename, file, config_file,
            'ruleSpaces': horizontal_rule_spaces,       # Bool
            'ruleRepetition': horizontal_rule_repeat,   # int
        }
-        remark_configs_plugins = {}
+        remark_lint_configs = {}


the rename should be a separate commit

yash-nisar · 2017-07-16T10:25:01Z

@Makman2 @sils How does it look now ?

Makman2 · 2017-07-17T13:02:08Z

bears/markdown/MarkdownBear.py

@@ -108,6 +110,9 @@ def create_arguments(filename, file, config_file,
            The number of times the horizontal rule character will be repeated.
        :param max_line_length:
            The maximum line length allowed.
+        :param check_valid_links:
+            Checks if links to headings and files in markdown point to
+            existing things.


Hmm that kinda sounds weird :D

Changed 👍

Makman2 · 2017-07-17T13:02:26Z

bears/markdown/MarkdownBear.py

@@ -65,7 +66,8 @@ def create_arguments(filename, file, config_file,
                         horizontal_rule: str='*',
                         horizontal_rule_spaces: bool=False,
                         horizontal_rule_repeat: int=3,
-                         max_line_length: int=None):
+                         max_line_length: int=None,
+                         check_valid_links: bool=None):


Use False instead of None, we don't need a tri-state for this field.

Makman2 · 2017-07-17T13:04:39Z

bears/markdown/MarkdownBear.py

@@ -136,13 +141,16 @@ def create_arguments(filename, file, config_file,
        # Remove { and } as remark adds them on its own
        settings = config_json[1:-1]

-        args = ['--no-color', '--quiet', '--setting', settings]
+        args = [filename, '--no-color', '--quiet', '--setting', settings]


do you really need filename, as the linter has use_stdin?

Yes, use_stdin doesn't work with validate-links 😄 I've removed the use_stdin though 👍

Ah write that into the commit message please :D
(the reason why you remove use_stdin)

Makman2 · 2017-07-17T13:06:36Z

tests/markdown/MarkdownBearTest.py

@@ -50,7 +50,7 @@


 @generate_skip_decorator(MarkdownBear)
-class MarkdownBearMaxLineLengthMessageTest(unittest.TestCase):
+class MarkdownBearTest(unittest.TestCase):


for me it's obvious, anyway put the reason into the commit message :)

Makman2 · 2017-07-17T13:13:33Z

bears/markdown/MarkdownBear.py

@@ -30,7 +30,7 @@ class MarkdownBear:

    _output_regex = re.compile(
        r'\s*(?P<line>\d+):(?P<column>\d+)'
-        r'\s*(?P<severity>warning)\s*(?P<message>.*)')
+        r'\s*(?P<severity>warning)\s*(?P<message>.+?)(?:  .*|\n|$)')


Please reason that inside the commit message :)

Actually I would rephrase the shortlog to something like Improve result message or so, as this is the effective reason :)

Makman2 · 2017-07-17T13:18:42Z

bears/markdown/MarkdownBear.py


        if max_line_length:
-            remark_configs_plugins['maximumLineLength'] = max_line_length
+            remark_lint_configs['maximumLineLength'] = max_line_length

        config_json = json.dumps(remark_configs_settings)


Maybe we should also rename this to remark_configs, so it's easier to distinct from remark_lint_configs?

Up to you 👍

One more commit ?

Done, added a commit stating the same 👍

AsnelChristian · 2017-07-18T11:01:46Z

please rebase and address @Makman2 issues :)

yash-nisar · 2017-07-18T13:37:35Z

Updated @Makman2

Makman2 · 2017-07-18T15:29:28Z

bears/markdown/MarkdownBear.py

@@ -65,7 +65,8 @@ def create_arguments(filename, file, config_file,
                         horizontal_rule: str='*',
                         horizontal_rule_spaces: bool=False,
                         horizontal_rule_repeat: int=3,
-                         max_line_length: int=None):
+                         max_line_length: int=None,
+                         check_valid_links: bool=False):


maybe just check_links? Strictly: As only checking valid links won't yield any results, as those are valid, right? ;)

Makman2 · 2017-07-18T15:36:25Z

For the rename commits: Please put the reason of the name-change into the commit body 👍

Add `validate-links` plugin and since the `validate-links` plugin is incompatible with stdin input, `use_stdin=True` was removed. Closes coala#924

Rename test classname from `MarkdownBearMaxLineLengthMessageTest` to `MarkdownBearTest` because there were a few generic additions to the tests.

Rename `remark_configs_plugins` to `remark_lint_configs` because the latter is more appropriate and future addition of plugins may conflict with the former name.

Improves the result message by modifying the regex and ignoring the non-crucial output obtained from the linter.

Rename to `remark_configs` so it becomes easier to distinguish from `remark_lint_configs`.

Makman2 · 2017-07-18T17:26:41Z

ack f68bd91 2a21048 d83fe5f 71f6bc5 d6100ce

Makman2 · 2017-07-18T17:29:46Z

@rultor merge

rultor · 2017-07-18T17:30:38Z

@rultor merge

@Makman2 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2017-07-18T17:32:24Z

@rultor merge

@Makman2 Done! FYI, the full log is here (took me 2min)

gitmate-bot added process/pending review size/S review carefully! labels Jul 14, 2017

sils reviewed Jul 15, 2017

View changes

Makman2 reviewed Jul 15, 2017

View changes

yash-nisar force-pushed the md-validate-links branch from 65183c5 to 95e0af0 Compare July 16, 2017 10:23

gitmate-bot added size/M and removed size/S labels Jul 16, 2017

yash-nisar force-pushed the md-validate-links branch from 95e0af0 to c1ef443 Compare July 16, 2017 11:54

Makman2 reviewed Jul 17, 2017

View changes

co-robo added process/wip and removed process/pending review labels Jul 18, 2017

yash-nisar force-pushed the md-validate-links branch from c1ef443 to fd503ca Compare July 18, 2017 13:37

gitmate-bot added process/pending review and removed process/wip labels Jul 18, 2017

Makman2 reviewed Jul 18, 2017

View changes

yash-nisar force-pushed the md-validate-links branch from fd503ca to 28be3bb Compare July 18, 2017 16:14

yash-nisar added 2 commits July 18, 2017 22:35

MarkdownBear: Add validate-links plugin

f68bd91

Add `validate-links` plugin and since the `validate-links` plugin is incompatible with stdin input, `use_stdin=True` was removed. Closes coala#924

MarkdownBear: Rename test classname

2a21048

Rename test classname from `MarkdownBearMaxLineLengthMessageTest` to `MarkdownBearTest` because there were a few generic additions to the tests.

yash-nisar added 3 commits July 18, 2017 22:37

MarkdownBear: Rename remark_configs_plugins

d83fe5f

Rename `remark_configs_plugins` to `remark_lint_configs` because the latter is more appropriate and future addition of plugins may conflict with the former name.

MarkdownBear: Improve result message

71f6bc5

Improves the result message by modifying the regex and ignoring the non-crucial output obtained from the linter.

MarkdownBear: Rename remark_configs_settings

d6100ce

Rename to `remark_configs` so it becomes easier to distinguish from `remark_lint_configs`.

yash-nisar force-pushed the md-validate-links branch from 28be3bb to d6100ce Compare July 18, 2017 17:08

Makman2 added process/approved and removed process/pending review labels Jul 18, 2017

rultor merged commit d6100ce into coala:master Jul 18, 2017

yash-nisar deleted the md-validate-links branch July 18, 2017 17:32

MarkdownBear: Add `validate-links` plugin #1919

MarkdownBear: Add `validate-links` plugin #1919

yash-nisar commented Jul 14, 2017 •

edited

yash-nisar commented Jul 14, 2017

sils Jul 15, 2017

Makman2 Jul 15, 2017

yash-nisar Jul 16, 2017 •

edited

Makman2 Jul 16, 2017

yash-nisar Jul 17, 2017

Makman2 Jul 15, 2017

yash-nisar Jul 16, 2017

Makman2 Jul 15, 2017

yash-nisar commented Jul 16, 2017 •

edited

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

Makman2 Jul 18, 2017 •

edited

yash-nisar Jul 18, 2017

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

Makman2 Jul 17, 2017

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

Makman2 Jul 17, 2017

yash-nisar Jul 18, 2017

yash-nisar Jul 18, 2017

AsnelChristian commented Jul 18, 2017

yash-nisar commented Jul 18, 2017

Makman2 Jul 18, 2017

yash-nisar Jul 18, 2017

Makman2 commented Jul 18, 2017

Makman2 commented Jul 18, 2017

Makman2 commented Jul 18, 2017

rultor commented Jul 18, 2017

rultor commented Jul 18, 2017

MarkdownBear: Add validate-links plugin #1919

MarkdownBear: Add validate-links plugin #1919

Conversation

yash-nisar commented Jul 14, 2017 • edited

Checklist

yash-nisar commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar Jul 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar commented Jul 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Makman2 Jul 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AsnelChristian commented Jul 18, 2017

yash-nisar commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Makman2 commented Jul 18, 2017

Makman2 commented Jul 18, 2017

Makman2 commented Jul 18, 2017

rultor commented Jul 18, 2017

rultor commented Jul 18, 2017

MarkdownBear: Add `validate-links` plugin #1919

MarkdownBear: Add `validate-links` plugin #1919

yash-nisar commented Jul 14, 2017 •

edited

yash-nisar Jul 16, 2017 •

edited

yash-nisar commented Jul 16, 2017 •

edited

Makman2 Jul 18, 2017 •

edited