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

Opening any LsL script causes error. #6

Closed
shadoskill opened this issue Apr 19, 2018 · 17 comments
Closed

Opening any LsL script causes error. #6

shadoskill opened this issue Apr 19, 2018 · 17 comments

Comments

@shadoskill
Copy link

shadoskill commented Apr 19, 2018

Actual behavior

SublimeLinter: #1 lslint sl_script_d0a7becf2e4425b8834708da06decc0c.lsl
ERROR: ===
Linter crashed.

Traceback

Traceback (most recent call last):
  File "...\Sublime Text 3\Installed Packages\SublimeLinter.sublime-package\lint/backend.py", line 100, in execute_lint_task
    errors = linter.lint(code, view_has_changed, settings) or []
  File "...\Sublime Text 3\Installed Packages\SublimeLinter.sublime-package\lint/linter.py", line 758, in lint
    cmd = self.get_cmd()
  File "...\Sublime Text 3\Installed Packages\SublimeLinter.sublime-package\lint/linter.py", line 482, in get_cmd
    if '@python' in which:
TypeError: argument of type 'property' is not iterable

Steps to reproduce

  • This is the first step.
    1. Open a LsL script

Environment

=BB= LSL:

    version: 2.2.0
    installed via Package Control: True

Sublime Text:

    channel: dev
    build: 3156
    portable: False
    platform: Windows
    architecture: x64
@buildersbrewery
Copy link
Owner

@shadoskill This is due to a breaking change within SublimeLinter 4. I'm currently reviewing whether or not the preprocessor linting is possible at all. For now, as a temporary workaround you can force usage of SublimeLinter 3. I know this answer isn't great, sadly my days are limited to 24h as well.

@buildersbrewery
Copy link
Owner

buildersbrewery commented Dec 12, 2018

Hello @kaste & @braver,

any pointers how to get this resolved? Compare code:

https://github.com/XenHat/SublimeLinter-contrib-lslint/blob/6674409/linter.py

@braver
Copy link

braver commented Dec 12, 2018

We have our documentation here: http://www.sublimelinter.com/en/stable/linter_attributes.html. There are probably a bunch of things that need to happen with that plugin. There has been an extensive beta period with a lot of communication earlier this year, and we did reach out to most plugins about the breaking changes. I guess you kinda missed that window. I'm not sure what you and @ XenHat already tried, or what's unclear... or are you looking for someone to rewrite the plugin for you?

@buildersbrewery
Copy link
Owner

@braver Using v4 we cannot define which nor run. We use which due to shipping binaries with the plugin (for users who don't have them in PATH) and we use run to run the linter with a preprocessor.

@braver
Copy link

braver commented Dec 13, 2018

We use which due to shipping binaries with the plugin

Yeah, that's not really something I would recommend doing. SublimeLinter shouldn't be the only way you can run a linter, it's merely a front-end. There's lots of ways to make stuff easier for end users, but you shouldn't ship linter executables with a SublimeLinter plugin. You'll notice every other plugin comes with an explanation on how to install the executable.

and we use run to run the linter with a preprocessor

Why, what does it do? If it does what I think it does, that won't work. If you take what's in the buffer, transform it via a preprocessor, and then send it to the linter, how do you take that feedback from the linter and relate it to what's in the buffer? Sublime and the linter will both contain different code and SublimeLinter will not be able to reliably present the feedback from the linter to the user. I guess now I understand some of what is going on in run, you're transforming the feedback back to the original code's layout?

@buildersbrewery
Copy link
Owner

buildersbrewery commented Dec 13, 2018

@braver

  • This plug-in defaults to using the binaries in PATH but falls back to using the included binaries when not being able to find any in PATH. This is for beginners who have problems setting up their PATH and to support portable installs on Windows where users don't want to mess with their PATH or simply can't (due to lack of rights).
  • See the GIF at README.md#linting for an example how this works or compare Enhancement request for preprocessors support XenHat/SublimeLinter-contrib-lslint#6 for the full story.

@kaste
Copy link

kaste commented Dec 13, 2018

That's just not proper python, @buildersbrewery. None of these methods are actually @classmethods and that's where the original error message comes from TypeError: argument of type 'property' is not iterable. You're accessing executable_path from the class, not from an instance. Linter.method is also not how you invoke methods, you use self.method.

executable_path has been deprecated and must be replaced with lslint iirc your linter binary name.

@kaste
Copy link

kaste commented Dec 13, 2018

Other than that the plugin probably works with our deprecation layers. Shipping binaries is totally ok. SublimeLinter doesn't really care.

@braver
Copy link

braver commented Dec 13, 2018

with our deprecation layers

Meaning it will probably break again in the future? ;)

I'm not saying SublimeLinter cares about much of what you're doing here, but this thing seems a bit convoluted compared to the capacity to maintain it all. Yes the platform allows you to override methods, but you then have to be able to keep up with changes that might break your overrides. If you can simplify what you're trying to do here, you'll have an easier time with it.

❤️ @kaste for the actual helpful details. I'm really no help at all here 😉

@buildersbrewery
Copy link
Owner

@kaste Thanks for the input, I believe I fixed everything now.

@kaste
Copy link

kaste commented Dec 13, 2018

👍

Since cmd is static, it is better to just define cmd = ('lslint', '-m', '-w', '-z', '-i') as a class member. SublimeLinter can apply optimizations for static cmds.

For searching a binary really use shutil.which

import shutil

def which(self, executable):
	enhanced_path = os.pathsep.join([
		os.environ.get('PATH', ''), 
		os.path.join(sublime.packages_path(), 'LSL', sublime.platform().strip()),
		os.path.join(sublime.packages_path(), 'LSL', 'bin', 'lslint', sublime.platform().strip()),
		os.path.join(sublime.packages_path(), 'LSL', 'bin', 'mcpp', sublime.platform().strip()),
		# etc ...
	])
	return shutil.which(executable, path=enhanced_path)

@buildersbrewery
Copy link
Owner

buildersbrewery commented Dec 13, 2018

@kaste for:

def which(self, executable):
    enhanced_path = os.pathsep.join(
        os.environ.get('PATH', ''),
        os.path.join(
            sublime.packages_path(),
            PKG_NAME,
            'bin',
            'lslint',
            sublime.platform() if sublime.platform() in ['linux', 'osx']\
                else 'windows' if platform.release() == 'XP'\
                else 'windows' + platform.architecture()[0][:-3]
        )
    )
    return shutil.which(executable, path=enhanced_path)

I receive a TypeError: which() missing 1 required positional argument: 'executable'. Looking at:

https://github.com/SublimeLinter/SublimeLinter/blob/f147f974321a227241db62c831958afda74491d5/lint/util.py#L168-L173

switching executable with cmd in the snippet above doesn't help either.

@kaste
Copy link

kaste commented Dec 13, 2018

My which was an alternative implementation of yours. It goes into the class as a method, then it receives self as first arg automatically.

SublimeLinter will call it once with 'lslint' and you can call it like self.which('mcpp') from run.

@kaste
Copy link

kaste commented Dec 13, 2018

Maybe return shutil.which(executable, path=enhanced_path) or super().which(executable) for extra compatibility.

@buildersbrewery
Copy link
Owner

@kaste thanks for the valuable feedback. Quick note: You had a small typo as it was os.pathsep.join([str, str, str]) not os.pathsep.join(str, str, str). Which I fixed in your comment for people who don't read the whole page.

@buildersbrewery
Copy link
Owner

@shadoskill I'll try to push an update later today.

@buildersbrewery
Copy link
Owner

buildersbrewery commented Jan 24, 2019

fixed in v4

@buildersbrewery buildersbrewery added this to the 4.0.0 milestone Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants