bears/pug: Add PugLintBear #1936

yash-nisar · 2017-07-17T11:08:49Z

Closes #290

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-17T11:09:02Z

CC @Makman2

Makman2 · 2017-07-17T13:21:27Z

bears/pug/PugLintBear.py

+    def process_output(self, output, filename, file):
+        stdout, stderr = output
+        yield from self.process_output_regex(stderr, filename, file,
+                                             self._output_regex)


why you don't use output_format=regex?

Makman2 · 2017-07-17T13:48:32Z

tests/pug/puglint_test_files/multiple_line_breaks.pug

+div
+
+
+div


are the missing \n at EOF on purpose?

Nope, my bad.

Fixed. 👍

AsnelChristian · 2017-07-18T09:37:48Z

bears/pug/PugLintBear.py

+        use_stderr=True)
+class PugLintBear:
+    """
+    A configurable linter and style checker for Pug (formerly Jade) that is a


"""Pug"'' (formerly "jade"

enclose them within backsticks

AsnelChristian · 2017-07-18T09:50:04Z

bears/pug/PugLintBear.py

+                        validate_extensions: bool=True,
+                        validate_self_closing_tags: bool=True,
+                        preferred_quotation: str='\'',
+                        max_lines_per_file: int=None,


change max_lines_per_file -> max_file_length, for consistency.

We have named the setting as max_lines_per_file in LineCountBear, so I think we should keep max_lines_per_file for consistency of the settings throughout.

there is the TailorBear that has max_file_length

Should we open an issue for that ?

@Makman2 , @yash-nisar let's vote 😄 and get rid of the other setting
i am for using max_file_length as we have max_line_length

~~Nvm, doing so right away~~ 😄

Haha okay, lets vote 👍

max_file_length might refer to the number of chars inside a document, max_lines_per_file is less ambigious^^ So imo max_lines_per_file is better 👍

AsnelChristian · 2017-07-18T09:51:48Z

bears/pug/PugLintBear.py

+            For example: If set to ``True``, prefer
+            ``input(type='text').class`` over ``input.class(type='text')``.
+        :param prohibit_class_literals_before_id_literals:
+            When ``True``, prefer all ID literals to be written before any


AsnelChristian · 2017-07-18T09:53:31Z

bears/pug/PugLintBear.py

+            When ``True``, all attributes should be written in lower case.
+            For example: If set to ``True``, prefer ``div(class='class')``
+            over ``div(Class='class')``.
+        :param require_lower_case_tags:


enforce_lower_case_tags

AsnelChristian · 2017-07-18T09:54:00Z

bears/pug/PugLintBear.py

+            When ``True``, all tags must be written in lower case.
+            For example: If set to ``True``, prefer ``div(class='class')``
+            over ``Div(class='class')``.
+        :param require_spaces_inside_attribute_brackets:


enforce_spaces_inside_attribute_brackets

AsnelChristian · 2017-07-18T09:54:32Z

bears/pug/PugLintBear.py

+        :param require_spaces_inside_attribute_brackets:
+            When ``True``, enforce space after opening attribute bracket and
+            before closing attribute bracket.
+        :param require_strict_equality_operators:


enforce_...

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

Updated @Makman2

Makman2 · 2017-07-18T15:15:22Z

bears/pug/PugLintBear.py

+                        validate_div_tags: bool=True,
+                        validate_extensions: bool=True,
+                        validate_self_closing_tags: bool=True,
+                        preferred_quotation: str='\'',


use "'" instead of '\''

Makman2 · 2017-07-18T15:19:59Z

bears/pug/PugLintBear.py

+            When ``True``, prefer all attribute blocks to be written before
+            any class literals.
+            For example: If set to ``True``, prefer
+            ``input(type='text').class`` over ``input.class(type='text')``.


what's with enforcing the other way around?

Makman2 · 2017-07-18T15:24:11Z

bears/pug/PugLintBear.py

+    def create_arguments(filename, file, config_file, puglint_config: str=''):
+        """
+        :param puglint_config:
+            The location of the ``.pug-lintrc`` config file.


Maybe

The location of a custom ``.pug-lintrc`` config file.

So it doesn't sound that this parameter is mandatory?

Sounds good 👍

yash-nisar · 2017-07-18T15:55:43Z

tests/pug/puglint_test_files/.pug-lintrc

+   "requireClassLiteralsBeforeIdLiterals":false,
+   "validateDivTags":true,
+   "disallowTagInterpolation":true
+}


Ah, eof damnit.

Makman2 · 2017-07-18T20:42:53Z

bears/pug/PugLintBear.py

+                        enforce_lower_case_attributes: bool=True,
+                        enforce_lower_case_tags: bool=True,
+                        require_spaces_inside_attribute_brackets: bool=False,
+                        require_strict_equality_operators: bool=True,


what's with these using enforce as prefix?

Okay, I'll use it for them too 👍

AsnelChristian · 2017-07-18T21:03:31Z

consistency 2017-07-18 21:42 GMT+01:00 Mischa Krüger <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In bears/pug/PugLintBear.py <#1936 (comment)>: > + prohibit_id_literals_before_attributes: bool=True, + prohibit_id_literals: bool=True, + prohibit_legacy_mixin_call: bool=True, + prohibit_multiple_line_breaks: bool=None, + prohibit_spaces_inside_attribute_brackets: bool=True, + prohibit_string_interpolation: bool=True, + prohibit_tag_interpolation: bool=True, + prohibit_specific_attributes: typed_list(str)=None, + prohibit_specific_tags: typed_list(str)=None, + enforce_class_literals_before_attributes: bool=False, + enforce_class_literals_before_id_literals: bool=False, + enforce_id_literals_before_attributes: bool=False, + enforce_lower_case_attributes: bool=True, + enforce_lower_case_tags: bool=True, + require_spaces_inside_attribute_brackets: bool=False, + require_strict_equality_operators: bool=True, what's with these using enforce as prefix? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1936 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ9xonk8d1-xuPtemc60Sp9mANVg2R7Rks5sPRjOgaJpZM4OZ1AM> .

AsnelChristian · 2017-07-18T21:05:47Z

we use more `enforce_` than `require_` 😅 2017-07-18 22:03 GMT+01:00 Ngoulla Asnel Christian <notifications@github.com

…

: consistency 2017-07-18 21:42 GMT+01:00 Mischa Krüger ***@***.***>: > ***@***.**** commented on this pull request. > ------------------------------ > > In bears/pug/PugLintBear.py > <#1936 (comment)>: > > > + prohibit_id_literals_before_attributes: bool=True, > + prohibit_id_literals: bool=True, > + prohibit_legacy_mixin_call: bool=True, > + prohibit_multiple_line_breaks: bool=None, > + prohibit_spaces_inside_attribute_brackets: bool=True, > + prohibit_string_interpolation: bool=True, > + prohibit_tag_interpolation: bool=True, > + prohibit_specific_attributes: typed_list(str)=None, > + prohibit_specific_tags: typed_list(str)=None, > + enforce_class_literals_before_attributes: bool=False, > + enforce_class_literals_before_id_literals: bool=False, > + enforce_id_literals_before_attributes: bool=False, > + enforce_lower_case_attributes: bool=True, > + enforce_lower_case_tags: bool=True, > + require_spaces_inside_attribute_brackets: bool=False, > + require_strict_equality_operators: bool=True, > > what's with these using enforce as prefix? > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#1936# pullrequestreview-50737361>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AJ9xonk8d1- xuPtemc60Sp9mANVg2R7Rks5sPRjOgaJpZM4OZ1AM> > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1936 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ9xopVPMwsQ7-VvyOHN9HlcVpOCqrbLks5sPR2kgaJpZM4OZ1AM> .

AsnelChristian · 2017-07-19T07:33:59Z

bears/pug/PugLintBear.py

+            ``input.class#id(type='text')``.
+        :param prohibit_class_literals:
+            When ``True``, disallow any class literals.
+            For example: If set to ``True``, prefer ``div(class='class')``


i am kind of lost prohibit_class_attribute_with_static_value default value is True so you make use of class literals but here prohibit_class_literals you prohibit them.... isn't that contradicting?

yash-nisar · 2017-07-19T11:24:22Z

One weird thing that I noticed was that the config (which is later converted to json) only takes the values true and null, so I've made the required changes.

yash-nisar · 2017-07-19T11:24:31Z

CC @Makman2 @AsnelChristian

Makman2 · 2017-07-19T13:21:57Z

bears/pug/PugLintBear.py

+            options = {
+                'disallowBlockExpansion':
+                    prohibit_block_expansion if prohibit_block_expansion
+                    else None,


can you maybe loop over the dict and assign None for each entry if needed?

options = {...} for k, v in options.items(): options[k] = v if v else None

or write a little function like map_bool()

I had initially done a settings_map where I mapped False to None

yash-nisar · 2017-07-20T06:57:26Z

@Makman2 Updated.

AsnelChristian · 2017-07-20T19:00:06Z

👍

AsnelChristian · 2017-07-20T19:00:49Z

ack 4cfe907

AsnelChristian · 2017-07-20T19:02:13Z

@Makman2 ping! 😄

Makman2 · 2017-07-20T22:56:23Z

@AsnelChristian bam! 💥
thanks for reviewing :D
rebase and lgtm @yash-nisar :)

Closes coala#290

yash-nisar · 2017-07-21T07:18:34Z

reack 1d31827

sils · 2017-07-21T07:18:46Z

@rultor merge

rultor · 2017-07-21T07:19:06Z

@rultor merge

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

rultor · 2017-07-21T07:21:21Z

@rultor merge

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

gitmate-bot added process/pending review size/XXL labels Jul 17, 2017

gitmate-bot added the review carefully! label Jul 17, 2017

Makman2 reviewed Jul 17, 2017

View reviewed changes

AsnelChristian reviewed Jul 18, 2017

View reviewed changes

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

yash-nisar force-pushed the pug-bear branch from 0a42ec2 to 56f8642 Compare July 18, 2017 12:13

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

Makman2 reviewed Jul 18, 2017

View reviewed changes

yash-nisar force-pushed the pug-bear branch from 56f8642 to 4134edd Compare July 18, 2017 15:49

yash-nisar commented Jul 18, 2017

View reviewed changes

yash-nisar force-pushed the pug-bear branch 2 times, most recently from 763e01b to d41309b Compare July 18, 2017 17:34

Makman2 reviewed Jul 18, 2017

View reviewed changes

AsnelChristian reviewed Jul 19, 2017

View reviewed changes

yash-nisar force-pushed the pug-bear branch from d41309b to 973a9b7 Compare July 19, 2017 11:23

Makman2 reviewed Jul 19, 2017

View reviewed changes

yash-nisar force-pushed the pug-bear branch from 973a9b7 to 4cfe907 Compare July 19, 2017 14:16

adhikasp approved these changes Jul 20, 2017

View reviewed changes

AsnelChristian added process/approved and removed process/pending review labels Jul 20, 2017

bears/pug: Add PugLintBear

1d31827

Closes coala#290

yash-nisar force-pushed the pug-bear branch from 4cfe907 to 1d31827 Compare July 21, 2017 06:35

gitmate-bot added the process/pending review label Jul 21, 2017

sils removed the process/pending review label Jul 21, 2017

rultor merged commit 1d31827 into coala:master Jul 21, 2017

yash-nisar deleted the pug-bear branch July 21, 2017 07:20

		div


		div

bears/pug: Add PugLintBear #1936

bears/pug: Add PugLintBear #1936

Conversation

yash-nisar commented Jul 17, 2017 • edited

Checklist

yash-nisar commented Jul 17, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar 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

yash-nisar commented Jul 18, 2017

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 via email

AsnelChristian commented Jul 18, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar commented Jul 19, 2017

yash-nisar commented Jul 19, 2017

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 20, 2017

AsnelChristian commented Jul 20, 2017

AsnelChristian commented Jul 20, 2017

AsnelChristian commented Jul 20, 2017

Makman2 commented Jul 20, 2017 • edited

yash-nisar commented Jul 21, 2017

sils commented Jul 21, 2017

rultor commented Jul 21, 2017

rultor commented Jul 21, 2017

yash-nisar commented Jul 17, 2017 •

edited

yash-nisar Jul 18, 2017 •

edited

Makman2 commented Jul 20, 2017 •

edited