-
Notifications
You must be signed in to change notification settings - Fork 18
Some improvements #63
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
base: dev
Are you sure you want to change the base?
Conversation
…k except for autocompletion
|
Hello @Phaiax, I've begin to look at your change yesterday and i'll try to test a look on windows and mac osx with your change this week-end. You're change are really interesting :) and address problems i was trying to solve (i was considering going full python by translating typescript language service in python, but that's quite a task and a bit of maintenance problem after, to solve the laguiness) if everything work well i'll merge asap. (i'll try to do that this week end) thx for your contribution :) |
|
In theory: Should the dev branch still work with sublime 2 or are all these ST3 statements just old crap?
|
|
From an end-user perspective, this PR is great and seems to solve a severe performance problem in T3S that was forcing me to not use it. However, it needs at least the following two changes, otherwise it is very broken: diff --git a/lib/Listener.py b/lib/Listener.py
index 51bc390..0fd15e6 100755
--- a/lib/Listener.py
+++ b/lib/Listener.py
@@ -137,7 +137,7 @@ class TypescriptEventListener(sublime_plugin.EventListener):
COMPLETION.trigger(view, TSS)
if not SETTINGS.get('error_on_save_only'):
- TSS.errors(view.file_name)
+ TSS.errors(view.file_name())
#debounce(TSS.errors, self.error_delay, 'errors' + str(id(TSS)), view.file_name())
diff --git a/lib/Utils.py b/lib/Utils.py
index 5285e50..ce39e33 100755
--- a/lib/Utils.py
+++ b/lib/Utils.py
@@ -156,7 +156,7 @@ def read_file(filename):
def read_and_decode_json_file(filename):
""" returns None or json-decoded file contents as object,list,... """
- f = read_file(f)
+ f = read_file(filename)
return json.loads(f) if f is not None else None
# FILE EXISTS |
|
i'll do the testing/merging next week as i'll be on holiday, sorry for the long delay. |
|
There is another thing I want to implement, the auto completer has to check wheather it's the same dot it belongs to. Thats sometimes confusing, but I will adress this issue during the next days. Ah, and the errorview to source linkage is misbehaving. Btw, I think its a good idea to indicate that there is a pending calculation of the current errors. Maybe I will use the first line of the error view to archive this. |
|
Here is another error that came up at some point which I assume is related to this PR: This happened after doing an F5 project refresh. Subsequent to that, when closing the project: |
|
I will hopefully fix that this weekend |
eg Outline view has to be moved in main group then restart of sublime then close outline window
|
I only experienced those import errors in files which are not imported by any other file. Those files will still be added and updated to the typescriptservice and I think it causes undefined behaviour. Is there any other case where those undefined imports can occour? There is also this other issue which has been in T3S before: you can define multiple root files but only the first one will ever be loaded or used. I think both problems are relatively easy to resolve, but may cause some bugs to appear which are harder to solve. Ill take a look. On July 24, 2014 5:28:37 PM CEST, Colin Snover notifications@github.com wrote:
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet. |
|
One way that seems to be possible to consistently reproduce the issue is:
With regards to the root files…I honestly don’t understand why they are a thing that exists. When I open a TypeScript file, I want code intel for that file. The files that need to load are any open TypeScript file, plus the opened files’ explicitly defined dependencies (/// comments, import statements). I don’t understand what circumstances exist that one should have to specify a “root” file (badly written code without explicit dependencies?). |
|
The root files are something that is related to typescript itself. If you type a compile command, you specify 'the root file'.
Because typescript allows for circular imports, there is no logical way to determine the correct root file. Your only could get a clue to the directory, if you try to find the directory where the imports are relative to. But how far should you search? And what if you first open a library file located somewhere else. So currently, if you open a typescript file and you did not specified or specified a non existent or wrong root file, it can't do anything. (The current problem is that he does something). So for your example of reproducing: Do you have a root file specified and does that root file import the file you open? Do you have a .sublime-project file? I will create some testprojects in the repository with all possible valid and invalid combinations just to be able to test. |
I believe the “root” file would be the file that is open in the currently focused tab. Circular dependencies between types are fine in TypeScript, even between those that are
The way the TypeScript compiler resolves dependencies is very straightforward, and if it doesn’t already, T3S should take the same approach to ensure correct interoperability. For absolute module IDs, the compiler looks for For relative module IDs, you just resolve the relative ID relative to the dirname of the dependent file, An approach where the open files in the editor are the ones that are used as dependencies for other open files in the editor leads to an incredibly brittle and badly functioning design, one that does not match how the compiler works and one that leads to false positives & false negatives in the editor.
I don’t know what this means; I have to do this all the time (using a bogus root file) and it pretty much works. I am a library developer; there is no “root file” for a library.
I have a .sublimets with a bogus root file. |
|
Ok, I've thought about it and tested what the typescript-services are capable of: At first It's no general problem to use the first open .ts file to init the typescript-services. The only drawback is the following situation: You have a (big) project and most of the files are closed. lets say the imports are as following: main.ts -> lib.ts -> utils.ts
The error would be recognized if the typescript-services were always initialized with the file from which all imports are starting. And I was able to reconstruct your error from above: Files: main.ts (root) >imports> lib.ts It only automatically adds files which have an import-line-of-sight to the "root"-file the typescript-services were inited with. There are two ways to resolve this:
So i draw the following conclusions. Correct me if you think i'm wrong.
so
I think that would be ok for everyone, even you as a library developer. Btw: you can take a look at the watched files aka the files in the import tree of typescript-services. I also fixed a bug which caused an error in when T3S tried to refer to the settings for a non-indexed .ts file. I will push soon. |
|
@Railk I would be pleased if you could comment my last comment. |
|
Sorry life happened, my free time went away and i've been busy since. |
|
@Phaiax Thanks for your thoughtful response. I’m planning to read through it in greater detail and respond to any points when I have a free moment. |
I can do this, but i'm in the usa currently without my laptop, so you'll have to wait until november.Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet. |
|
@Phaiax thanks, that would be great 👍 |
|
ping |
|
I'm trying it in the next few days, but no promise :D |
|
pong |
Jep i know.. I'm participating in an Hackathon on Friday so its done this weekend probablyDiese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet. |
|
Hey I've created a new plugin, which is based on T3S and includes these patches. https://github.com/Phaiax/ArcticTypescript You can install ArcticTypescript using PackageControl. Please remove T3S first, both plugins together are incompatible.
Phaiax |
Hey
I've worked a little bit on the code in the dev branch.
Former i had the problem, that there was a bunch of unexecuted showError commands in the async tss queue if I type a character just every debounce_delay seconds. Every showError takes 5-10 seconds on my project so it could easily take up to a minute until i could see the most recent errors. This took CPU time and slowed the builds also.
I did a lot, these are the main changes:
This results in a much better responsiveness. Previously i had some delays (gui hang ups) on completion trigger etc, but now it works much smoother.
More changes:
All these changes are working well with Linux and sublime 3.
When i'm done i can test it with sublime 2 (if they have a free version).
Can you test it in windows?
I'm open to any suggestions on my changes.
regargs, Dän