These are chat archives for ceylon/ceylon-ide-eclipse

22nd
Mar 2016
Gavin King
@gavinking
Mar 22 2016 09:38
So, @davidfestal it seems to me that our IDE is currently broken :-(
David Festal
@davidfestal
Mar 22 2016 09:39
strange,I've not pushed anything on master since several days
Gavin King
@gavinking
Mar 22 2016 09:39
I get an error at startup, and then basic functionality doesn’t work
David Festal
@davidfestal
Mar 22 2016 09:39
what is the error ?
I'm working on a branch for build abstraction
David Festal
@davidfestal
Mar 22 2016 09:56
If you copy the error in a gist I might be able to help without having to switch my current branch ;-)
hopefully
Bastien Jansen
@bjansen
Mar 22 2016 10:01
so, I'm making AbstractRefactoring.editorData and EditorData.node non-optional, because if they are missing there's no way we can do the refactoring
that should make things easier
I'll also replace IEditorPart with CeylonEditor to avoid that non-sense assert (is CeylonEditor editorPart);
David Festal
@davidfestal
Mar 22 2016 10:03
beware, I'm not sure there are not some cases when we might perform a refactoring on something that is not the CeylonEditor
CeylonSourceViewer for example ?
Maybe I'm saying nonsense
but I remember when abstracting this that there was way more use-cases than expected
quite unexpected ones also
Bastien Jansen
@bjansen
Mar 22 2016 10:04
we do asserts anyway, so it would fail
David Festal
@davidfestal
Mar 22 2016 10:04
so if Gavin confirms it's not used in any other context
OK, if you see it I believe you
Bastien Jansen
@bjansen
Mar 22 2016 10:09
EclipseExtractValueRefactoring does not have that assertion
but I suppose it's always called with a CeylonEditor, I'll have to check
David Festal
@davidfestal
Mar 22 2016 10:10
yes, you 'd better check IMO
FTR I took a long time looking into the code before finding all the special-cases that make thigs so complicated
but afair there was a reason
at least I thought there was a room left for a use outside of CeylonEditor
Bastien Jansen
@bjansen
Mar 22 2016 10:12
@gavinking do you know cases where a refactoring would be called on a IEditorPart which is not a CeylonEditor?
Gavin King
@gavinking
Mar 22 2016 10:19
@davidfestal it’s always a CeylonEditor
David Festal
@davidfestal
Mar 22 2016 10:19
Strnage how the Java code was then
but it's too old now, I cannot remember exactly what made me think so
but if you say so it's good news
David Festal
@davidfestal
Mar 22 2016 15:44
@gavinking : after our discussion about the abstraction, and abstracting the environment instead of the common part
I started refactoring this way in my work area (model management, build, virtual file system), and it allowed cleaning quite well already
silly to have not seen things like this before
Bastien Jansen
@bjansen
Mar 22 2016 15:47
@davidfestal I added importProposals to PlatformServices
but I don't really like the unsafeCast() :/
David Festal
@davidfestal
Mar 22 2016 15:47
shhh
don't say it :-)
I din't like it either
but at least it's just at one place
Bastien Jansen
@bjansen
Mar 22 2016 15:48
I suppose it will break if we accidentally use different type arguments?
Gavin King
@gavinking
Mar 22 2016 15:49
@davidfestal great
Bastien Jansen
@bjansen
Mar 22 2016 15:49
(in classes that will use PlatformServices)
David Festal
@davidfestal
Mar 22 2016 15:49
and allows having access to these services globally
Gavin King
@gavinking
Mar 22 2016 15:50
@davidfestal what is the reason for the IDE locking up when you save a Ceylon file during a build?
I think we should be able to avoid that behavior
David Festal
@davidfestal
Mar 22 2016 15:50
@bjansen : I added a better way to have access to the services in a typesafe way
but it's still on a branch
Gavin King
@gavinking
Mar 22 2016 15:50
we don’t stop you from editing during a build, so why should we stop you from saving?
David Festal
@davidfestal
Mar 22 2016 15:50
I'll merge as soon as I have tested nothing critical is broken
Bastien Jansen
@bjansen
Mar 22 2016 15:51
ok
great
David Festal
@davidfestal
Mar 22 2016 15:52
@gavinking : it's related to the fact that in Eclipse, the save action is performed under a scheduling rule that conflicts with the workspace shceduling rule used by the incremental project builder
When the abstraction is finished
we could leave the binary generation inside the builder
and do the central model typechecking outside a scheduling rule
however we would have many things to check
because now the use of the workspace scheduling rule prevents us from many concurrency bugs
Gavin King
@gavinking
Mar 22 2016 15:54
because frankly this is one of the worst things in the IDE
and very much contributes to the impression of bad performance
David Festal
@davidfestal
Mar 22 2016 15:54
but on the other hand it introduces quite a number of blocking behaviors
I know this
This is mostly related to the fact that we do everything inside the Incremental project builder
Gavin King
@gavinking
Mar 22 2016 15:54
the build process should never block the UI
i know i’ve said it before
David Festal
@davidfestal
Mar 22 2016 15:54
and it's "Eclipse"
Gavin King
@gavinking
Mar 22 2016 15:55
but it’s still not fixed :)
David Festal
@davidfestal
Mar 22 2016 15:55
As I said, once the build abstraction is available it should give the opportunity to do better
Gavin King
@gavinking
Mar 22 2016 15:55
ok
David Festal
@davidfestal
Mar 22 2016 15:55
For IntelliJ and other IDEs I have to manage a build state myself
Gavin King
@gavinking
Mar 22 2016 15:56
couldn’t we temporarily solve the problem by doing saves in a Job?
David Festal
@davidfestal
Mar 22 2016 15:56
with a list of changes, a list of pending files for the next typechecking and the next binary generation, etc ...
Gavin King
@gavinking
Mar 22 2016 15:56
currently the editor does it synchronously, IIRC
David Festal
@davidfestal
Mar 22 2016 15:56
And I din this with immutable collections
Gavin King
@gavinking
Mar 22 2016 15:56
and sure, I know that is the user expectation
David Festal
@davidfestal
Mar 22 2016 15:56
in order to avoid excessive synchronization
Gavin King
@gavinking
Mar 22 2016 15:56
but still
really, if we solve this, the complaints about performance will probably go away
David Festal
@davidfestal
Mar 22 2016 15:58
Sure
That along with indexing-based search
Gavin King
@gavinking
Mar 22 2016 15:58
it’s probably the worst bug we have in all of ceylon
David Festal
@davidfestal
Mar 22 2016 15:58
yeah
Gavin King
@gavinking
Mar 22 2016 15:58
sure, Open Declaration is unacceptable slow
but I don’t have to use it
I can use Open Resource
David Festal
@davidfestal
Mar 22 2016 15:58
sure I agree
@gavinking what we could do currently is:
cancel the pending build to save a file
However that will then start a full build the next time
afair
Stéphane Épardaud
@FroMage
Mar 22 2016 16:00
sounds horrible
David Festal
@davidfestal
Mar 22 2016 16:00
but I would have to check
Gavin King
@gavinking
Mar 22 2016 16:00
@davidfestal well that’s probably not the right way either
David Festal
@davidfestal
Mar 22 2016 16:00
I know
Gavin King
@gavinking
Mar 22 2016 16:00
the right way would be to remember the list of changed files when cancelling the build
David Festal
@davidfestal
Mar 22 2016 16:00
but then It's quite proper Eclipse stuff
yes, but for the moment the list of changed files is given by the Incremental builder project deltas
and if you flush the deltas
and don't do anything
then you have lost the previous changes
certainly there are things we could do
but I still think that doing part of this outside the incremental project builder and possibly reducing the scope of the incremental project builder shceduling rule is the solution
Gavin King
@gavinking
Mar 22 2016 16:04
well we just keep them somewhere else!
“just"
David Festal
@davidfestal
Mar 22 2016 16:04
I'll keep them in the abstracted CeylonProjectBuild obect
as well as the related logic
that's why I say that it will be easier after the abstraction
since we should be (at least in theory) able to manage the build delta ourselves
based on ResourceChangeListeners
outside the incremental project builder
that's the aim yes
at least that would be possible
though a sensilble refactoring
Gavin King
@gavinking
Mar 22 2016 16:24
@bjansen your fixes worked, very nice
does that mean intellij has an Inline refactoring now?
Bastien Jansen
@bjansen
Mar 22 2016 16:24
yes
Gavin King
@gavinking
Mar 22 2016 16:24
and Extract Function / Extract Parameter ?
Bastien Jansen
@bjansen
Mar 22 2016 16:24
I'm trying to add a popup asking if it should replace all the occurrences
but I came across that hierarchy issue
Gavin King
@gavinking
Mar 22 2016 16:25
ok
Bastien Jansen
@bjansen
Mar 22 2016 16:25
those two will come later
Gavin King
@gavinking
Mar 22 2016 16:25
how much later? :)
Bastien Jansen
@bjansen
Mar 22 2016 16:26
dunno, are they needed for a first release?
Gavin King
@gavinking
Mar 22 2016 16:26
well they are done in common
should be easy to wire them in, no?
Bastien Jansen
@bjansen
Mar 22 2016 16:26
probably yes
Gavin King
@gavinking
Mar 22 2016 16:26
you don’t have to add the popups stuff if you don’t have time
they work without them
just copy/paste whatever code you already have for Extract Value
should be trivial
there is a teensy bit of code to copy/paste from EclipseExtractFunctionRefactoring too
but nothing time-consuming
I did all the hard work for you
Bastien Jansen
@bjansen
Mar 22 2016 16:29
ok
David Festal
@davidfestal
Mar 22 2016 16:33
@gavinking : couuld you open an issue about the save/build conflict that blocks the UI ?
and assign it to me in High-prio ?
Gavin King
@gavinking
Mar 22 2016 16:33
we don’t have one?
David Festal
@davidfestal
Mar 22 2016 16:33
not sure
not a specific one afaik
Gavin King
@gavinking
Mar 22 2016 16:35
ok
Gavin King
@gavinking
Mar 22 2016 16:42
@bjansen be very, very careful about translating substring() to span()
span() never returns an empty string
Bastien Jansen
@bjansen
Mar 22 2016 16:42
span(start, stop - 1)
what should I use instead ?
Gavin King
@gavinking
Mar 22 2016 16:43
depends
spanTo(), initial(), terminal(), measure(), depending on the situation
there is no exact equivalent
i have often wondered if we should add substring(), just to make translating Java code easier
Bastien Jansen
@bjansen
Mar 22 2016 16:44
well I only use span() if start > 0 && stop < str.length
otherwise I use spanFrom() or spanTo()
Gavin King
@gavinking
Mar 22 2016 16:44
no you used it in InlineRefactoring
where stop==start
Bastien Jansen
@bjansen
Mar 22 2016 16:46
in InterpolationVisitor?
Gavin King
@gavinking
Mar 22 2016 16:47
yes
i’m fixing it to use measure()
but I think we should add a substring()
just to avoid this kind of bug
when translating Java
David Festal
@davidfestal
Mar 22 2016 16:47
I also wondered several times about this one
Bastien Jansen
@bjansen
Mar 22 2016 16:47
hm ok