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

Added SublimeLinter-contrib-csl for Citation Style Language #106

Merged
merged 3 commits into from
May 5, 2021

Conversation

Marcool04
Copy link
Contributor

Hi,
I just thought I'd share this linter I created to integrate a shell script that validates Citation Style Language (CSL), a specific schema of XML.
Thanks in advance for your reviews, advice and possible inclusion.
PS: I'm sorry about the name, I didn't realize all contribs should include SublimeLinter-contrib-name, and named my repo SublimeLinter-csl 😕 Is that a problem?

@kaste
Copy link
Contributor

kaste commented May 3, 2021

I don't find the csl-validator.sh file in the https://github.com/citation-style-language/utilities repo. Where are my 👓?

You could probably rename the repo to match our contrib-xxx style.

@Marcool04
Copy link
Contributor Author

I don't find the csl-validator.sh file in the https://github.com/citation-style-language/utilities repo. Where are my 👓?

Haha, your eyes are just fine, it' hasn't been upstream-ed yet citation-style-language/utilities#40
I've been using it from my own version, and didn't really notice the problem… This is probably a bit premature isn't it! Silly me…

You could probably rename the repo to match our contrib-xxx style.

I'll do that. Would your prefer I just close this PR, wait for the citation-style-language/utilities to merge (or not, and perhaps offer a version that uses a repo from my github…) and then re-open a new PR? Or just keep this open in the interim?

@Marcool04
Copy link
Contributor Author

Fixed the missing -contrib- bit…

@kaste
Copy link
Contributor

kaste commented May 3, 2021

Hm, looks like csl-validator.sh is a stand alone program/script. Maybe you could just distribute it with the plugin? (Unlikely a user might have it in their PATH, no?)

@Marcool04
Copy link
Contributor Author

Oh sure, if that's something that's common why not! I was just trying to figure out the most "logical" location for it. The idea for the script comes from utilities maintained by that org, so I thought if it was in one of their repos (the aptly named "utilities" one!) then it would have more legitimacy from (possible) user's perspective…

@kaste
Copy link
Contributor

kaste commented May 3, 2021

Yeah sure, but then we have to wait for the merge, the repo has no commits since 2017. Maybe they waited just for your PR.

@Marcool04
Copy link
Contributor Author

Ah, you do have good eyes…
That might indeed be a better solution. Could you point out an example of another linter plugin that includes its own external script please? I'm just trying to figure out how relative/absolute paths should work in such a case… Thanks in advance.

@kaste
Copy link
Contributor

kaste commented May 4, 2021

I don't have an example plugin. Assuming csl-validator.sh is just a sibling file, I think it's as easy as:

import os

linter_script = os.path.join(os.path.dirname(__file__), "csl-validator.sh")

class ...:
    cmd = (linter_script,)

If that works:

Somehow tell git to add csl-validator.sh as an executable file, t.i. with setting file attributes.

Create an empty file .no-sublime-package also as a sibling, t.i. at the root of the Sublime plugin, to prevent it gets installed as a zip file.

Marcool04 added a commit to Marcool04/SublimeLinter-contrib-csl that referenced this pull request May 4, 2021
Manual: "If including executables or shared libraries, add a .no-sublime-package
file. Adding this file to the root of your repository will prevent
Package Control from keeping your package packed as a .sublime-package
file in ST3."

See also SublimeLinter/package_control_channel#106 (comment)
@Marcool04
Copy link
Contributor Author

Marcool04 commented May 4, 2021

  1. Worked fine (once I noticed the comma in cmd = (linter_script,) that is needed so that file paths aren't split on spaces!)
  2. git update-index --chmod=+x csl-validator.sh
  3. Done!

There you go :)

@kaste
Copy link
Contributor

kaste commented May 4, 2021

Yeah, that looks okay.

  1. contrib-csl comes before contrib-css... here in this PR
  2. Obviously, the README is outdated and you need a new version then already. I would point out in install.txt that jq and curl are required. (Since your only doing a http request in the sh file, implementing it in the plugin - in python that is - would be better of course but 🤷 for now it's what you got.)

To put the linter_script = ... in the body of the class confused me although it works; I would declare it as a global variable.

@Marcool04
Copy link
Contributor Author

Marcool04 commented May 4, 2021

I agree with you entirely that at this point, it would make a lot more sense to have the whole http request and json decode in the linter. I'm absolutely no python dev and I apologize in advance for anything weird, but I came up with this:

import requests
from SublimeLinter.lint import PythonLinter

class SublimeLinterContribCsl (PythonLinter):
    cmd = None
    regex = r'^M="(?P<message>.+)" L=(?P<line>\d+) C=(?P<col>\d+)$'
    defaults = {
        'selector': 'source.csl, text.xml'
    }
    def run(self, cmd, code):
        url = "https://validator.w3.org/nu/"
        files = { 'file': ('style.csl', code) }
        data = {
            'schema': 'https://raw.githubusercontent.com/citation-style-language/schema/v1.0.1/csl.rnc https://raw.githubusercontent.com/citation-style-language/schema/602ad40976b7b455a3ce0b79f5534e8e75f088e9/csl.sch',
            'parser': 'xml',
            'laxtype': 'yes',
            'level': 'error',
            'out': 'json',
            'showsource': 'no'
        }
        response = requests.post(url, data=data, files=files)
        json = response.json()
        for error_report in json['messages']:
            return('M="{}" L={:d} C={:d}'.format(error_report['message'].encode('utf-8'), error_report['firstLine'], error_report['firstColumn']))

The above run() function works in a standalone script, but this show up when I try it in linter.py:

Traceback (most recent call last):
  File "/Applications/Sublime Text.app/Contents/MacOS/sublime_plugin.py", line 123, in reload_plugin
    m = imp.reload(m)
  File "./python3.3/imp.py", line 276, in reload
  File "<frozen importlib._bootstrap>", line 584, in _check_name_wrapper
  File "<frozen importlib._bootstrap>", line 1022, in load_module
  File "<frozen importlib._bootstrap>", line 1003, in load_module
  File "<frozen importlib._bootstrap>", line 560, in module_for_loader_wrapper
  File "<frozen importlib._bootstrap>", line 868, in _load_module
  File "<frozen importlib._bootstrap>", line 313, in _call_with_frames_removed
  File "/Users/mark/Library/Application Support/Sublime Text 3/Packages/SublimeLinter-contrib-csl/linter.py", line 1, in <module>
    import requests
ImportError: No module named 'requests'

So obviously, the requests module is not loaded in the env the plugin is run in… is there a best practice to fix this? I have searched around quite a bit, the closest I came to any similar issue is this: https://stackoverflow.com/questions/29250870/how-can-i-import-a-python-module-in-a-sublime-text-plugin but the solution is ad hoc and not suitable for distribution. The solution offered to this problem: https://stackoverflow.com/questions/15180537/how-to-include-third-party-python-packages-in-sublime-text-2-plugins, re-packaging requests (and its 4 other dependencies) seemed kind of weird to me really. I tried copying over the required folders from my /usr/local/lib/python3.9/site-packages anyway just to see, but that failed too:

 File "/Users/mark/Library/Application Support/Sublime Text 3/Packages/SublimeLinter-contrib-csl/requests-dist/urllib3/util/ssl_.py", line 381, in ssl_wrap_socket
    # not be using SNI anyways due to IP address for server_hostname.
TypeError: load_verify_locations() takes at most 2 arguments (3 given)

So, I guess I'm just going about this the wrong way. Do I need to call out from the plugin framework through the use of the cmd attribute in any case, even to get python code to run? In which case bash, python, whatever the interpreter doesn't make much difference does it… Replacing a curl dependency with a python-requests (and its own dependencies) one is barely a win, is it?

Sorry this is getting longer and longer… thank you for your patience.

@kaste
Copy link
Contributor

kaste commented May 4, 2021

Okay down the rabbit hole. It is basically not possible to not use requests for uploading files using the standard Python3.3 library. You can add requests as a dependency in Sublime by creating just another file dependencies.json at the root of the plugin again with the following contents:

{
   "*": {
      "*": [
         "requests"
      ]
   }
}

As a developer you have to call Package Control: Satisfy Dependencies from the Command Palette (ctrl+shift+p) after that and/or restart Sublime Text once/twice. Check the Sublime console (View -> Show Console from the menu).

The return type of run is str. The following should be the better starting point:

        return "\n".join(
            'M="{}" L={:d} C={:d}'.format(
                error_report['message'].encode('utf-8'), 
                error_report['firstLine'], 
                error_report['firstColumn']
            ) for error_report in json['messages']
        )

If that works, better though you return just the text() here and then implement find_errors

from SublimeLinter.lint import LintMatch
import json
import logging

logger = logging.getLogger('SublimeLinter.plugin.csl')


...
    def find_errors(self, output):
        """Parse errors from linter's output."""
        try:
            content = json.loads(output)
        except ValueError:
            logger.error(
                "JSON Decode error: We expected JSON from 'csl', "
                "but instead got this:\n{}\n\n"
                .format(output)
            )
            self.notify_failure()
            return

        for error in content['messages']:
            yield LintMatch(
                match=error,
                line=error['firstLine'] - 1,  # apply line_col_base manually
                col=error['firstColumn'] - 1,
                message=error['message'],
                error_type='error',   #  does 'csl' have different error types like 'warning'?
                code='',  # does 'csl' output different rule names?
            )

@Marcool04
Copy link
Contributor Author

Wow you're a great teacher thanks for taking the dive!
I've attempted an implementation as a pure python self-contained linter: https://github.com/Marcool04/SublimeLinter-contrib-csl following your advice. I had to work around a few corner cases (the messages don't always have the same keys…) Seems to work like that now. To answer your comment-questions:

  • I used self.line_col_base[0] and self.line_col_base[1], hope that's right…
  • There are no other types of errors than errors (no warnings etc.) as far as I can tell.
  • I don't think there are any kinds of error codes or anything like that.

Two notes : I also had to import logging to get logger to work. Maybe the internet offline check is overkill?

@kaste
Copy link
Contributor

kaste commented May 5, 2021

Yes, you need to import logging but I did include this in my snippet. Note though that you cannot just use __name__ here but SublimeLinter.plugin.csl. As we're in a shared environment here you must use a "namespaced" string.

The offline check is overkill, what you probably should do is to add a timeout to the post call, post(..., timeout=5).

@kaste
Copy link
Contributor

kaste commented May 5, 2021

From #106 (comment) "contrib-csl comes before contrib-css... here in this PR" so you need to edit this PR before we can ship.

@Marcool04
Copy link
Contributor Author

From #106 (comment) "contrib-csl comes before contrib-css... here in this PR" so you need to edit this PR before we can ship.

Oh… i had done, just forgot to push: bd2ed90

@Marcool04
Copy link
Contributor Author

Yes, you need to import logging but I did include this in my snippet.

Oh so you did! Sorry my eyes aren't working…

Note though that you cannot just use __name__ here but SublimeLinter.plugin.csl. As we're in a shared environment here you must use a "namespaced" string.

Ok, fixed.

The offline check is overkill, what you probably should do is to add a timeout to the post call, post(..., timeout=5).

I've implemented timeout and put in two exception handlers for timeout and connection error. I used a rather high timeout (60 sec) on the read timeout because the server can be quite slow at times… I made the connection error an info otherwise the console opens and is a bit disruptive if you want to work offline.

@kaste
Copy link
Contributor

kaste commented May 5, 2021

Just make a new release. EDIT: LGTM! 😁

@kaste kaste merged commit a0fae26 into SublimeLinter:master May 5, 2021
@Marcool04
Copy link
Contributor Author

Just make a new release.

Done!

Thanks again @kaste for this interesting initiation to writing SublimeLinter plugins! You rock! 🎸
Not sure how useful it'll be to others, but you sure taught me a few things!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants