-
Notifications
You must be signed in to change notification settings - Fork 16
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
Misc improvements #47
Conversation
The caches are *very* expensive to compute so we should not trash them on every start of Sublime. We of course trash everything if the plugin gets uninstalled, otherwise we clean if the last access time (`st_atime`) is older than 14 days. A couple of tricks here: - The `except NameError` trick keeps the cache warm if SublimeLinter (or this plugin) reloads. Otherwise Python would clean up the directories. - If you exit Sublime, it is always a fast exit. (Sublime wants to be fast!) And so the temporary directories are kept. However, on a cold load `tmpdirs` is obviously empty, we thus need to repopulate it. Unfortunately we cannot just pass strings around because Python would trash every `TemporaryDirectory` immediateley on garbage collection. So we still store these objects, and resurrected directories become `FakeTemporaryDirectory` just for the interface of having a `.name`.
Remove the trailing `:`, these are probably typos although SublimeLinter is permissive here. Remove `--incremental` which is outdated. Users can still provide this option using `args`.
mypy actually operates with caches (probably even using sqlite) and these aren't locked while running. This leads to various errors when we try to run it concurrently. SublimeLinter itself has no flag which forbids concurrent processing, so we just lock it ourself. Creating and acquiring a lock using a `defaultdict` is in itself not thread aware but it is simple enough here.
The user can set `cache-dir` to `False` to omit the arg from the command completely. This is especially useful if the actual cache dir is configured in the `mypy.ini` file. Omit automatically in case the default location `.mypy_cache` exists. Likely it is populated and the right thing. In all cases do not manually edit `cmd`. Just set the setting and let SublimeLinter do the rest.
Thanks for these changes. I didn't even realize Could you update the README for these changes as well?
These were documented features for the defaults dictionary, but they might have been removed since. I don't quite remember what exactly they did, however. |
Also, you say that SL can interpret filenames in the output by itself, but do we want to change the default for |
Ah, the README, will take a look. The trailing We could of course change the default of |
Mypy does have an incremental mode that runs in the background, but is harder to inferface with and I hope it hasn't been removed yet, but in the current world of LSP implementations and live analysis, I doubt that. This is tracked in #32. |
I noticed I actually run a different version of this plugin since a long time now. Sorry, this is a rather unfocused PR.
Improvements/changes:
If there is a
.mypy_cache
folder in the project just use (respect) that.Do not check a falsy value for
cache-dir
. Leave the automatic default to''
, but if the user sets explicitlyFalse
, respect that. (Typical in the mypy.ini we can say where the cache is located, so it is good to somehow disable the automatic behavior.)If there is no working dir (t.i. no open folder for that window typically) do not lint at all. (This fixes an edge case really, when
working_dir
isNone
we don't have a stable key fortmpdirs
and hence the chance that unrelated stranded files share the same cache folder.)Do not clean all cache folder on reload which is triggered on startup, and for example when SublimeLinter hot reloads. The caches are very expensive to compute, and so we try to keep them if possible (if the cache is used within the last 14 days). (That's a tricky first commit here, and I hope I didn't messed it up while squashing and preparing this PR.)
Capture the error-
code
andfilename
.code
is supported via mypy'sshow-error-codes
flag (maybe roughly a year old),filename
is supported by SublimeLinter core also since a year or so. (That also means users can actually follow imports now. SublimeLinter accepts errors from multiple files in one go.)Allow only one mypy process per cache (basically per project or working dir). This is a bug fix; SublimeLinter actually tries to run mypy on multiple files in the background, but mypy does not lock the caches. It is strictly forbidden to run mypy on the same folder concurrently, so we ensure that with a lock. (This is cheaply implemented but just enough since we're not "highly" concurrent here in this context, usually these processes would just overlap a bit.)