SublimeLinter / SublimeLinter-gcc

This linter plugin for SublimeLinter provides an interface to gcc or other gcc-like (cross-)compiler.
MIT License
22 stars 5 forks source link

Adapt to new SL api #15

Closed sknat closed 5 years ago

sknat commented 5 years ago

This changes the configuration, going from cpp_executable to gcc.executable & g++.executable. This should allow cleaner code than parsing the result of get_syntax imho

jfcherng commented 5 years ago

I have no time to test this PR yet but I believe there are actually many people who are more suitable than me to maintain/dev this package :D

Here's my two cents.

cixtor commented 5 years ago

@sknat aside from some KeyError exceptions triggered in def cmd(self) the rest seems to work fine. Can you please include the following changes in your pull-request? This patch will allow people who have not configured the linter yet to work with files using the default settings.

diff -u -r old/linter.py new/linter.py
--- old/linter.py 2019-07-23 10:04:50.000000000 -0700
+++ new/linter.py 2019-07-23 10:00:22.000000000 -0700
@@ -108,16 +108,24 @@
         We override this method, so we can change executable, add extra flags
         and include directories basing on settings.
         """
-        extra_flags = self.settings['extra_flags']
+        extra_flags = []
+        executable = 'gcc'
+        include_dirs = []
+        if 'extra_flags' in self.settings:
+            extra_flags = self.settings['extra_flags']
+        if 'executable' in self.settings:
+            executable = self.settings['executable']
+        if 'include_dirs' in self.settings:
+            include_dirs = self.settings['include_dirs']
         if isinstance(extra_flags, list):
-          extra_flags = ' '.join(extra_flags)
+            extra_flags = ' '.join(extra_flags)
         return self.cmd_template.format(
-            executable = self.settings['executable'],
+            executable = executable,
             common_flags = ' '.join(self.common_flags),
             extra_flags = apply_template(extra_flags),
             include_dirs = apply_template(' '.join({
                 '-I' + shlex.quote(include_dir)
-                for include_dir in self.settings['include_dirs']
+                for include_dir in include_dirs
             })),
             garbage_file = shlex.quote(self.garbage_file),
         )

Example of Exception

Traceback (most recent call last):
  File "SublimeLinter.sublime-package/lint/backend.py", line 157, in execute_lint_task
    errors = linter.lint(code, view_has_changed)
  File "SublimeLinter.sublime-package/lint/linter.py", line 1010, in lint
    cmd = self.get_cmd()
  File "SublimeLinter.sublime-package/lint/linter.py", line 712, in get_cmd
    cmd = cmd()
  File "SublimeLinter-gcc.sublime-package/linter.py", line 111, in cmd
    extra_flags = self.settings['extra_flags']
  File "SublimeLinter.sublime-package/lint/linter.py", line 225, in __getitem__
    raise KeyError(key)
KeyError: 'extra_flags'

Also, minor nitpick, but it seems like the indentation in def get_executable() is broken.

kaste commented 5 years ago

Well guys, this plugin re-implements a lot of the SublimeLinter core API which is not good.

  1. Remove the SublimeLinter v3 compatibility
  2. There is actually no behavior attached to default_settings. Everything goes into defaults
  3. You cannot subclass Gcc. All linter inherit from Linter (or another built-in Linter supertype).
  4. The name executable is used by SublimeLinter core and cannot change its meaning here. http://www.sublimelinter.com/en/stable/linter_settings.html#executable

Please take a look at the clang plugin. https://github.com/SublimeLinter/SublimeLinter-clang/blob/master/linter.py

E.g.

class Clang(Linter):
    cmd = 'clang ${args} -'
    defaults = {
        'selector': 'source.c',
        'args': '-Wall -fsyntax-only -fno-caret-diagnostics',
        '-I +': [],
        '-x': 'c'
    }
    regex = OUTPUT_RE
    multiline = True
    on_stderr = None

What we have here as 'include_dirs' is actually built-in functionality via -I +. (The only missing part would be that there is no name mapping implemented so the user has to use 'I' as the name in the settings just like on the command line. Otherwise this handles single values and arrays.)

On cmd you can set any typical arg which is then static in the sense that a user can never change that. For clang we don't use this, instead we set a sane default for args which can be completely modified via settings. x is also configurable by users but also has a sane default so it appears as '-x c' on the command line. For this plugin, args is the same as your extra_flags.

Note that cmd, args and basically everything else allows variable substitution. Your project_folder is actually not thread aware because it accesses active_window() directly. There are a number of placeholders/variables defined, see http://www.sublimelinter.com/en/stable/settings.html#settings-expansion. For your use-case, 'project_folder' is probably expressed as ${folder:file_path}. E.g. the user can just set "I": ["$folder/foo", "~/bar"] and it will just work.

As I said, it might be enough to copy paste clang and modify the regex, and set a different '-x' and 'args' default.

EDIT:

If you absolutely need a new substitution var, you need to compute and set it on self.context['myvar'] ='foo'. You cannot change a pre-defined var, only add new ones.

The garbage file computing for Windows is probably not thread aware, maybe it is okay. Iirc you would set a default '-o': 'dev/null', and then maybe in cmd() do self.settings['o'] = '...'.

jfcherng commented 5 years ago

I actually hope it support binaries other than gcc, i.e., https://github.com/SublimeLinter/SublimeLinter-gcc/issues/1. There are many gcc-derived compilers which are not named simply gcc but they are basically sharing everything in command line.

I believe that a full copy from the Clang linter would be a good start.

kaste commented 5 years ago

But executable is built-in. A user can set it to a string or an array of strings and it will replace the first thing we define here in cmd. So when we do a simple declarative gcc $args anyone can change the 'gcc' in here effectively to either 'my-super-gcc $args' or even 'a-wrapper -foo gccc $args'. Is that enough, or do you need more functionality here.

EDIT: Well for SL3 you had to built this for yourself, but if we drop SL3, SL4 has all of this.

jfcherng commented 5 years ago

The garbage file computing for Windows is probably not thread aware, maybe it is okay. Iirc you would set a default '-o': 'dev/null', and then maybe in cmd() do self.settings['o'] = '...'.

Just a FYI. The garbage file is not important indeed. I add them because of https://github.com/SublimeLinter/SublimeLinter-gcc/issues/4. There are some warnings/errors that will not be caught with -fsyntax-only and only emitted in a real compilation phase. To do a compilation, it would be better provide gcc an output file path (otherwise the output file will be in the same directory with the source).

jfcherng commented 5 years ago

@cixtor @sknat

Could you help test codes SL4 branch. It runs successfully on my PC. The linter settings need to be changed (see https://github.com/SublimeLinter/SublimeLinter-gcc/tree/SL4#settings).

cixtor commented 5 years ago

I just tested branch SL4 on some of my projects and it works well.

jfcherng commented 5 years ago

@kaste Thanks for the detailed information.

@sknat Thank you but I have this PR closed because we are in favor of https://github.com/SublimeLinter/SublimeLinter-gcc/pull/17.