WIP: XMLBear: Support for style specifications #1187

namanyadav12 · 2016-12-21T19:00:09Z

A format can be specified in the arguments or in the configuration.
The valid args are c14n, c14n11, exc-c14n and oldxml10.
If a style is specified, then the style messages from the bear have MAJOR severity.

Closes #1098

gitmate-bot · 2016-12-21T19:00:23Z

Thanks for your contribution!

Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.

I have rebased properly. Please see this for a tutorial.
I have gone through the commit guidelines and I've followed them:
I have followed the guidelines for the commit shortlog.
I have followed the guidelines for the commit body.
I have included the issue URL.
I have used the Fixes keyword if the commit fixes a real bug and the Closes keyword if it adds a feature/enhancement. See more here.
I have run all the tests and they all pass. That includes that all CI (below) is green - if GitMate issues appear you will have to amend your commits, pushing new commits on top is not sufficient!

As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well!

jayvdb · 2016-12-22T05:31:27Z

bears/xml2/XMLBear.py

                         xml_schema: path='',
-                         xml_dtd: path_or_url=''):
+                         xml_dtd: path_or_url='',
+                         xml_style: str=''):


instead of str it should be a restricted list of values checked with a function like path_or_url.

And default should be None, not ''

jayvdb · 2016-12-22T05:32:59Z

bears/xml2/XMLBear.py

        """
        args = (filename,)
        if xml_schema:
            args += ('-schema', xml_schema)
        if xml_dtd:
            args += ('-dtdvalid', xml_dtd)
+        styles = ('c14n', 'c14n11', 'exc-c14n', 'oldxml10')


CONSTANT list, should be a module or class attribute

jayvdb · 2016-12-22T05:36:18Z

tests/xml2/XMLBearTest.py

+    XMLBear,
+    valid_files=(valid_xml_file,),
+    invalid_files=(invalid_xml_chars,),
+    settings={'xml_style': 'oldxml10'},


This style is not being tested. This test class is identical to XMLBearCorrectedTest , etc.
You need to find an XML style issue, and ensure that the linter bear reports the error that xmllint emits and that its severity is correct.

jayvdb · 2016-12-22T05:36:45Z

tests/xml2/XMLBearTest.py

+    settings={'xml_style': 'oldxml10'},
+    tempfile_kwargs={'suffix': '.xml'})
+
+XMLBearInvalidStyleTest = verify_local_bear(


XMLBearInvalidStyleTest -> XMLBearUnrecognisedStyleNameTest

jayvdb · 2016-12-22T05:38:48Z

bears/xml2/XMLBear.py

+            args += ('--' + xml_style,)
+            self._diff_severity = RESULT_SEVERITY.MAJOR
+        elif xml_style != '':
+            self.warn('Invalid xml_style argument ' + xml_style +


this should be an error -- the user has mistyped the style. coala should not allow invalid configuration.

jayvdb · 2016-12-22T05:39:50Z

tests/xml2/XMLBearTest.py

+    XMLBear,
+    valid_files=(valid_xml_file,),
+    invalid_files=(invalid_xml_chars,),
+    settings={'xml_style': 'wrong args'},


need to confirm that this error is the cause of the failure.
otherwise, some other error could occur and this test look like it has been effective.

gitmate-bot · 2016-12-31T01:05:26Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

sils · 2017-01-01T13:03:52Z

bears/xml2/XMLBear.py

+    """
+    style = str(xml_style)
+    if style not in XMLBear._styles:
+        logging.error('Unrecognised style ' + style + '. Valid xml'


I think this should be a warning and you should mention that you just ignore the argument because you're not erroring out, you're just ignoring that part right?

sils · 2017-01-01T13:04:37Z

bears/xml2/XMLBear.py

        """
        :param xml_schema: ``W3C XML Schema`` file used for validation.
        :param xml_dtd:    ``Document type Definition (DTD)`` file or
                           url used for validation.
+        :param xml_style:  ``XML Style Specification`` Relevant args are
+                           c14n, c14n11, exc-c14n and oldxml10.


can you include a URL where I can find out more about those cryptic things? Can we maybe ake them less cryptic?

you need to include that info here so the user can find out more about the specs, not me

oh, ok !! I misunderstood you earlier

sils · 2017-01-01T13:05:50Z

bears/xml2/XMLBear.py

        """
        args = (filename,)
        if xml_schema:
            args += ('-schema', xml_schema)
        if xml_dtd:
            args += ('-dtdvalid', xml_dtd)
+        if xml_style:
+            args += (xml_style,)
+            self._diff_severity = RESULT_SEVERITY.MAJOR


why is this major? Also if you do this, all the other errors in the diff will have a major severity as well, right? That sounds bad.

This was given in the issue by @jayvdb, you can check it out here #1098

namanyadav12 · 2017-01-03T10:32:29Z

@sils you can find out more about the formats at
https://www.w3.org/TR/2001/REC-xml-c14n-20010315
https://www.w3.org/TR/xml-c14n11/
https://www.w3.org/TR/xml-exc-c14n/

namanyadav12 · 2017-01-03T10:34:46Z

Also @sils, self._diff_severity = RESULT_SEVERITY.MAJOR is to make sure that If the user has chosen a style in the configuration, the style messages from the bear are treated as "MAJOR" severity errors.
This was given in the issue by @javydb

NiklasMM · 2017-01-03T18:55:09Z

Please include the links that you gave in the comment, as sils pointet out here

NiklasMM · 2017-01-03T18:55:25Z

54f9232 needs work

A format can be specified in the arguments or in the configuration. The valid args are c14n, c14n11, exc-c14n and oldxml10. If a style is specified, style issues have a MAJOR severity. Closes coala#1098

gitmate-bot · 2017-02-12T02:26:09Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-02-19T12:12:41Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-02-26T21:59:27Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-03-06T07:46:33Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-03-13T17:33:28Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-03-21T03:19:55Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-03-28T02:00:14Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-04T00:40:18Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

jayvdb · 2017-04-05T12:09:57Z

bears/xml2/XMLBear.py

@@ -68,7 +91,7 @@ def process_output(self, output, filename, file):
                    output_regex=self._output_regex),
                self.process_output_corrected(
                    stdout, filename, file,
-                    diff_severity=RESULT_SEVERITY.INFO,
+                    diff_severity=self._diff_severity,


Isnt this a thread unsafe operation?
Is that a problem? cc: @Makman2

in praxis it's not (due to python executing everything in several processes), though this is a local variable and shall be used as such. So replace self._diff_severity with just diff_severity 👍

jayvdb · 2017-04-11T01:09:44Z

We've all invested a lot of time into this PR.
Please come back and complete this PR, when you have time.

gitmate-bot · 2017-04-20T16:40:33Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-28T02:27:27Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-28T02:27:28Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-05T01:07:32Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-12T17:03:58Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-19T15:43:46Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-27T01:30:46Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-03T00:10:26Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-10T09:57:04Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-17T08:37:02Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-24T07:17:25Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-01T05:57:24Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-08T04:37:11Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-08T04:37:11Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-15T03:17:27Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-22T01:57:23Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot added process/pending review size/S labels Dec 21, 2016

jayvdb reviewed Dec 22, 2016

View reviewed changes

jayvdb requested changes Dec 22, 2016

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 22, 2016

namanyadav12 force-pushed the xml_style branch from 9fe080e to 749fa9e Compare December 23, 2016 15:48

gitmate-bot added process/pending review size/M and removed process/wip size/S labels Dec 23, 2016

namanyadav12 changed the title ~~XMLBear: Support for style specifications~~ [WIP}XMLBear: Support for style specifications Dec 31, 2016

namanyadav12 changed the title ~~[WIP}XMLBear: Support for style specifications~~ [WIP]XMLBear: Support for style specifications Dec 31, 2016

namanyadav12 force-pushed the xml_style branch from 749fa9e to 2e47414 Compare December 31, 2016 07:11

namanyadav12 changed the title ~~[WIP]XMLBear: Support for style specifications~~ XMLBear: Support for style specifications Dec 31, 2016

sils reviewed Jan 1, 2017

View reviewed changes

sils added process/wip and removed process/pending review labels Jan 1, 2017

namanyadav12 force-pushed the xml_style branch from 2e47414 to 54f9232 Compare January 3, 2017 10:31

gitmate-bot added process/pending review and removed process/wip labels Jan 3, 2017

XMLBear: Support for style specifications

26b9437

A format can be specified in the arguments or in the configuration. The valid args are c14n, c14n11, exc-c14n and oldxml10. If a style is specified, style issues have a MAJOR severity. Closes coala#1098

jayvdb reviewed Apr 5, 2017

View reviewed changes

namanyadav12 closed this Apr 8, 2017

jayvdb changed the title ~~XMLBear: Support for style specifications~~ WIP: XMLBear: Support for style specifications Apr 11, 2017

jayvdb reopened this Apr 11, 2017

gitmate-bot added the status/STALE label Aug 31, 2017

WIP: XMLBear: Support for style specifications #1187

Are you sure you want to change the base?

WIP: XMLBear: Support for style specifications #1187

Conversation

namanyadav12 commented Dec 21, 2016

gitmate-bot commented Dec 21, 2016

jayvdb Dec 22, 2016 • edited Loading

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

gitmate-bot commented Dec 31, 2016

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

namanyadav12 commented Jan 3, 2017

namanyadav12 commented Jan 3, 2017

NiklasMM commented Jan 3, 2017

NiklasMM commented Jan 3, 2017

gitmate-bot commented Feb 12, 2017

gitmate-bot commented Feb 19, 2017

gitmate-bot commented Feb 26, 2017

gitmate-bot commented Mar 6, 2017

gitmate-bot commented Mar 13, 2017

gitmate-bot commented Mar 21, 2017

gitmate-bot commented Mar 28, 2017

gitmate-bot commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Apr 11, 2017

gitmate-bot commented Apr 20, 2017

gitmate-bot commented Apr 28, 2017

gitmate-bot commented Apr 28, 2017

gitmate-bot commented May 5, 2017

gitmate-bot commented May 12, 2017

gitmate-bot commented May 19, 2017

gitmate-bot commented May 27, 2017

gitmate-bot commented Jun 3, 2017

gitmate-bot commented Jun 10, 2017

gitmate-bot commented Jun 17, 2017

gitmate-bot commented Jun 24, 2017

gitmate-bot commented Jul 1, 2017

gitmate-bot commented Jul 8, 2017

gitmate-bot commented Jul 8, 2017

gitmate-bot commented Jul 15, 2017

gitmate-bot commented Jul 22, 2017

jayvdb Dec 22, 2016 •

edited

Loading