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

17th
Mar 2016
Gavin King
@gavinking
Mar 17 2016 10:22
OK, so this is intolerable guys
I have been wrestling for 4 days to port and clean up one file to ide-common
it’s the most painful thing I’ve had to do in this project
we need to rethink the approach we’re taking here
which is to say: we need to start thinking before activating Paste Java to Ceylon
Bastien Jansen
@bjansen
Mar 17 2016 10:23
what kind of problems did you have?
Gavin King
@gavinking
Mar 17 2016 10:23
and we need to fix this utterly intolerable experience
where the IDE locks up for 10 seconds every time i save a file
@davidfestal get rid of that bullshit ant task
it’s awful
look, a line-by-line literal port of mostly-bad Java code to Ceylon is producing even-worse Ceylon
we need to stop that
and we need to start thinking about creating robust abstractions
not just introducing bullshit interfaces in a totally adhoc manner
and actually have a proper notion of layers where in each layer we know what level of abstraction we are working at
Bastien Jansen
@bjansen
Mar 17 2016 10:26
if you have better design ideas, I'd like to know them
Gavin King
@gavinking
Mar 17 2016 10:26
well, sure
for example
NodePrinter is absolute trash
so is EditorData
what level of abstraction is nodes operating at?
is it IDE-independent or not?
why is there no proper abstraction of the token stream?
why do we have useless interfaces that declare nothing?
Bastien Jansen
@bjansen
Mar 17 2016 10:28
nodes should be mostly IDE-independent
Gavin King
@gavinking
Mar 17 2016 10:28
nodes.text() does not appear to be
Bastien Jansen
@bjansen
Mar 17 2016 10:28
it wasn't always the case because our subclasses of PhasedUnit didn't exist yet
Gavin King
@gavinking
Mar 17 2016 10:28
since it seems that there is some totally implicit assumption somewhere that i might not have a token stream
which i assume results from the fact that on IntelliJ tokens have some sort of different representation
in which case that should have been abstracted
Bastien Jansen
@bjansen
Mar 17 2016 10:29
"might not have"?
it's not an optional param
Gavin King
@gavinking
Mar 17 2016 10:29
well I don’t know
in intellij are tokens a List<CommonToken>?
Bastien Jansen
@bjansen
Mar 17 2016 10:30
of course, I'm using the same parser
shared String text(Node term, JList<CommonToken> tokens) {
ok, the signature of shared JIterator<CommonToken>? getTokenIterator(JList<CommonToken>? tokens, DefaultRegion region) is likely wrong
it shouldn't be optional in this one
I suppose this is because the Java impl was checking for a null list, so I translated it like that
Gavin King
@gavinking
Mar 17 2016 10:31
ok that’s good: so in that case NodePrinter is utterly superfluous
that’s good, since I already deleted it :)
so we had a bullshit single-method interface that was totally unnecessary
another huge issue we have lurking: all the positions and lengths we pass around are wrong now, AFAICT
since they are calculated from the lengths of Ceylon Strings
but they are passed to Java code which indexes according to the UTF-16 encoding
Bastien Jansen
@bjansen
Mar 17 2016 10:34
I think NodePrinter used to be necessary because my plugin was lacking something, although I can't recall what exactly, but it might be useless now
Gavin King
@gavinking
Mar 17 2016 10:35
so I’m sure everything breaks as soon as you have code with surrogate pair characters in it
probably not the worst bug ;)
Bastien Jansen
@bjansen
Mar 17 2016 10:35
yeah I haven't tested funky characters, so I have no idea what will happen
Gavin King
@gavinking
Mar 17 2016 10:36
everything breaks, surely
@bjansen try pulling and see how much I just broken intellij
Bastien Jansen
@bjansen
Mar 17 2016 10:39
so all that ranting was for a single useless interface? :)
Gavin King
@gavinking
Mar 17 2016 10:39
The really huge problem I have here is a hate these interfaces with shared formal variable members
no
it’s for the whole design of the refactoring stuff
in Ceylon, initialization is supposed to be very controlled
we’re not supposed to have variable members
Bastien Jansen
@bjansen
Mar 17 2016 10:39
I don't really like those variables either
Gavin King
@gavinking
Mar 17 2016 10:39
except in extreme circumstances
because if you go down that path, you wind up having to give all these members optional types
and then you are in a world of pain of assert (exists …) which is exactly what I see here
i improved it somewhat
Bastien Jansen
@bjansen
Mar 17 2016 10:41
but then again, we currently have one refactoring that has been abstracted (well, 2 now), it was mainly a PoC, we abstracted a lot of other useful things so it was pretty obvious from the start that we'd have to rethink refactorings a little before making more
Gavin King
@gavinking
Mar 17 2016 10:41
i dunno. it was the first thing i touched
and it’s really a mess
i admit that the previous Java code was far from perfect
Bastien Jansen
@bjansen
Mar 17 2016 10:42
if you had tried to abstract a quick fix instead, things would have been way easier
Gavin King
@gavinking
Mar 17 2016 10:42
it was initializing start in a way i don’t like doing in Java
but that is a far worse thing to do in Ceylon
@bjansen perhaps
but it still would have taken me 10 seconds every time I saved or ran a refactoring
while this awful Ant task runs
i can’t work like that
Bastien Jansen
@bjansen
Mar 17 2016 10:44
what does the ant task have to do with the way we abstract features?
Gavin King
@gavinking
Mar 17 2016 10:44
nothing: i’m saying that the whole experience is intolerable
it can’t take four days to port one refactoring
Bastien Jansen
@bjansen
Mar 17 2016 10:44
eh, I suppose you don't even have ceylon-ide-intellij open
Gavin King
@gavinking
Mar 17 2016 10:44
nope
Bastien Jansen
@bjansen
Mar 17 2016 10:45
code completion sometimes takes minutes to show up
Gavin King
@gavinking
Mar 17 2016 10:45
ok, sure, i cleaned some things up, and i fixed a couple of pre-existing bugs
it’s worse that I have to wait for the stupid ant task to finish in order to run a refactoring
seriously
Rename is almost unusable
try renaming two things, one after the other
Bastien Jansen
@bjansen
Mar 17 2016 10:47
that's what @jvasileff experienced IIRC: after using a refactoring, it's possible you lose modifications you made while things were building
Gavin King
@gavinking
Mar 17 2016 10:48
yes, this happened to me once
i had to go back to the local history to recover all my changes
momentarily scary
Bastien Jansen
@bjansen
Mar 17 2016 10:49
so yeah, the IDE has serious performances problems in some cases
but that's not new
Gavin King
@gavinking
Mar 17 2016 10:49
but this has nothing to do with the ceylon build
Bastien Jansen
@bjansen
Mar 17 2016 10:49
I've been whining about this for a year
Gavin King
@gavinking
Mar 17 2016 10:49
the build of the project finishes
then the fucking Ant task runs and forces some other shit to build that I don’t even care about
Bastien Jansen
@bjansen
Mar 17 2016 10:51
i'm not even sure this task is useful, unless you modify things in module.ceylon
why would eclipse dependencies change if you modify something totally unrelated?
Gavin King
@gavinking
Mar 17 2016 10:53
how can i turn it off?
ceylon/ceylon-ide-eclipse#1734
one for you, perhaps @bjansen ?
Bastien Jansen
@bjansen
Mar 17 2016 10:53
in the project settings > builders
you can uncheck it
Gavin King
@gavinking
Mar 17 2016 10:53
unless it was a pre-existing bug, in which case perhaps it’s mine
which project? not common...
Bastien Jansen
@bjansen
Mar 17 2016 10:54
the eclipse plugin
Gavin King
@gavinking
Mar 17 2016 10:54
Update Eclipse Dependencies?
Bastien Jansen
@bjansen
Mar 17 2016 10:54
I think the task is supposed to download jars from the eclipse SDK or something like that
yeah
Gavin King
@gavinking
Mar 17 2016 10:54
there’s a whole bunch of builders there
ok, indeed, that is a lot better, it seems
@davidfestal does CeylonParseController wait for the binary generation to finish? or does it go ahead and do its thing as soon as the project is typechecked?
Bastien Jansen
@bjansen
Mar 17 2016 10:59
ceylon/ceylon-ide-eclipse#1734 is a pre-existing bug ;)
Gavin King
@gavinking
Mar 17 2016 11:00
ok
Bastien Jansen
@bjansen
Mar 17 2016 11:00
I suppose it could add parentheses around the expression if it involves operators?
Gavin King
@gavinking
Mar 17 2016 11:00
well, that’s the crappy fix
Bastien Jansen
@bjansen
Mar 17 2016 11:00
:D
Gavin King
@gavinking
Mar 17 2016 11:08
ceylon/ceylon-ide-eclipse#1735
this one is definitely new
so @bjansen when you want to test changes to ide-common
can you do that w/o doing a full rebuild of all Ceylon?
Bastien Jansen
@bjansen
Mar 17 2016 11:10
of course
Gavin King
@gavinking
Mar 17 2016 11:10
right
what i though
and what’s the minimal thing necessary for that?
Bastien Jansen
@bjansen
Mar 17 2016 11:10
just run ant publish ide-quick in ceylon-ide-common
then
refresh the project com.redhat.ceylon.ide.common-1.2.3.car and you're good to go
com.redhat.ceylon.ide.common-1.2.3.car is the only proxy project you should need open
Gavin King
@gavinking
Mar 17 2016 11:11
ok, good
Gavin King
@gavinking
Mar 17 2016 12:33
@bjansen hrm, no, that does not seem to work
oh, wait
David Festal
@davidfestal
Mar 17 2016 12:34
Hi @bjansen and @gavinking
Bastien Jansen
@bjansen
Mar 17 2016 12:34
hi
David Festal
@davidfestal
Mar 17 2016 12:34
As I said on the ceylon gitter channel
I wasn't available this morning
sorry about that
Gavin King
@gavinking
Mar 17 2016 12:35
np
David Festal
@davidfestal
Mar 17 2016 12:35
I read all the discussions ;-)
sorry about the Ant Task. It doesn't bring any problem or slowness here and I didn't know you were still having it run automatically at each incremental build
Bastien Jansen
@bjansen
Mar 17 2016 12:36
well it's enabled by default
David Festal
@davidfestal
Mar 17 2016 12:36
@gavinking to answer to your question about the CeylonParseController, it waits to the source model being available yes
but that doesn't include binary generation
only typechecking afair
Gavin King
@gavinking
Mar 17 2016 12:55
ok, so refactorings, etc, should be available while binary generation is still happening?
have we really tested that?
David Festal
@davidfestal
Mar 17 2016 12:55
it seems to me yes
Gavin King
@gavinking
Mar 17 2016 13:29
what is the purpose of the ModelJ2C interface?
David Festal
@davidfestal
Mar 17 2016 13:29
This is specific to the java 2 Ceylon 2 Java interop we use in the Eclipse IDE
if some error is left inn the Java code, then all the calls from Java code to Ceylon code
may fail with a declaration not found returned from the backend
this is a limitation of the current java 2 Ceylon 2 java interop in the Java backend
So this interface allows supressing all the Java 2 Ceylon calls
while having the Ceylon code being compiled
Gavin King
@gavinking
Mar 17 2016 13:32
sounds like the Ceylon source should be in a separate project
David Festal
@davidfestal
Mar 17 2016 13:32
it's just a structure that allows easier debug / error search in some cases
no no
Gavin King
@gavinking
Mar 17 2016 13:32
with no dependency to the java source
David Festal
@davidfestal
Mar 17 2016 13:32
not possible for now
there are too much inter-dependencies in the existing code
I tried believe me
please let me finish the current abstraction of the incrementalbuild
before touching this
I agree in a further step
we might separate things more
As you can see:
I already pushed all the Java -> Ceylon calls into the the J2C classes
and they are all in a separate source directory
and existing (old) Java source is in a separate directory
and Ceylon code is in a separate source directory
and for now, if we respect this structure this works pretty well
Bastien Jansen
@bjansen
Mar 17 2016 13:36
except for those 4 strange errors that appear every time we touch ide-common
David Festal
@davidfestal
Mar 17 2016 13:36
right, @bjansen
I didn't have time to look at them
but you just have to save modelJ2C to make them disappear
not too hard ;-)
IMO
but again please don't touch this structure until I finish the abstraction
Bastien Jansen
@bjansen
Mar 17 2016 13:37
yes I know the workaround
David Festal
@davidfestal
Mar 17 2016 13:38
by the way, in the Java backend bi-directional interop is intended to be enhanced and made easier
in the future
(there are 2 issues for this I think)
which will be good for us
Gavin King
@gavinking
Mar 17 2016 13:57
what on earth is all this stuff about:
shared interface NewNameRefactoring {
    shared variable formal String? internalNewName;
    shared default String newName
            => synchronize {
        on = this;
        function do() {
            if (exists n=internalNewName) {
                return n;
            } else {
                internalNewName = initialNewName;
                assert (exists n = internalNewName);
                return n;
            }
        }
    };
    assign newName {
        synchronize {
            on = this;
            void do() {
                internalNewName = newName;
            }
        };
    }
    shared formal String initialNewName;
    shared formal Boolean forceWizardMode;
}
WTF?
David Festal
@davidfestal
Mar 17 2016 13:58
It's just an abstraction of code written in Java
Gavin King
@gavinking
Mar 17 2016 13:58
why did we have synchronization in the java code?!
David Festal
@davidfestal
Mar 17 2016 13:58
you have some refactrings that allow creating a new name
Well, probably a way to manage a variable attribute in an interface
@gavinking : it's maybe an abuse and you will certainly do things better
remember so of this code was written by ceylon beginners ;-)
no seriously, the idea of all this is to use mixins
David Festal
@davidfestal
Mar 17 2016 14:03
so to be able to just satisfy NewNameRefactoring, be able to use the newName attribute (along with its internal state) , while extending the AbstractRefactring class
that, in the case of Eclipse at least, finally extends the Eclipse Refactoring class
since we cannot have multiple inheritance
Gavin King
@gavinking
Mar 17 2016 14:05
i think all this mixin inheritance is an abuse
David Festal
@davidfestal
Mar 17 2016 14:05
we used (abused ?) interfaces to be able to add such aspects to the main refactirng class along the type hirarchy
If you say it
at least you can better understand why we did so
Bastien Jansen
@bjansen
Mar 17 2016 14:06
we need mixins because very often, the concrete class needs to extend a specific class from Eclipse SDK or IntelliJ SDK
Gavin King
@gavinking
Mar 17 2016 14:11
but why is extension the right thing to use here?
i would not use inheritance for a lot of this stuff
you’re moving the abstraction into the client code
when it should be abstracting over aspects of the environment
that’s wrong
David Festal
@davidfestal
Mar 17 2016 14:12
can you explain a bit more ?
Gavin King
@gavinking
Mar 17 2016 14:13
what is the thing that changes?
David Festal
@davidfestal
Mar 17 2016 14:13
surely we have general things to learn if terms of design
Gavin King
@gavinking
Mar 17 2016 14:13
  • the refactoring?
  • or the things it depends on?
Bastien Jansen
@bjansen
Mar 17 2016 14:13
the things it depends on
Gavin King
@gavinking
Mar 17 2016 14:13
right
precisely
so what are we abstracting?
Bastien Jansen
@bjansen
Mar 17 2016 14:14
the things it depends on
Gavin King
@gavinking
Mar 17 2016 14:14
right
so where does the inheritance belong?
clearly: in the model of the environment
but you have no strong model of the bits of the environment
instead, you’ve modeled this as being different kinds of refactorings
an “eclipse refactoring” and an “intellij refactoring"
that’s bogus because you’re generating N*M different implementations
N=number of IDEs
M=number of refactorings
Bastien Jansen
@bjansen
Mar 17 2016 14:16
then how do you suggest we improve that?
Gavin King
@gavinking
Mar 17 2016 14:17
you have N different concrete implementations of each “service” provided b the environment
this is a really classic abuse of inheritance
and it’s why people say idiotic stuff like “prefer delegation to inheritance"
it’s precisely this case that they’re thinking of when they say that idiotic stuff
in fact, what they mean is: use inheritance to model the right things
David Festal
@davidfestal
Mar 17 2016 14:19
so you're saying that all the formal methods that we have only to provide IDE-specific implems should be made available as a service outside the abstracted classes ?
Gavin King
@gavinking
Mar 17 2016 14:19
right
David Festal
@davidfestal
Mar 17 2016 14:19
I understand what you mean
Gavin King
@gavinking
Mar 17 2016 14:19
you need finer-grained abstractions
or, better: finer-grained reusable abstractions
Bastien Jansen
@bjansen
Mar 17 2016 14:20
@davidfestal isn't this what you did with platformUtils?
David Festal
@davidfestal
Mar 17 2016 14:20
a bit of it yes
probably
and for the model abstraction
Gavin King
@gavinking
Mar 17 2016 14:20
where the abstracted things get plugged in as services in the constructor
Bastien Jansen
@bjansen
Mar 17 2016 14:20
where IDEs have to register their own "service"
Gavin King
@gavinking
Mar 17 2016 14:20
i don’t want to start talking about dependency injection — ‘cos it’s rubbish
David Festal
@davidfestal
Mar 17 2016 14:21
I understand fully
what you're saying
Gavin King
@gavinking
Mar 17 2016 14:21
but think about the way you assemble services in DI
David Festal
@davidfestal
Mar 17 2016 14:21
this should be quite straight forward to change for the Ceylonprojects, Ceylonproject, etc ...
classes
OK, we'll try to have a look how things can be enhanced this way
Bastien Jansen
@bjansen
Mar 17 2016 14:23
so for example, the current DocumentChanges interface wouldn't be a mixin anymore, but rather a constructor parameter?
Gavin King
@gavinking
Mar 17 2016 14:24
@bjansen well that may be too fine grained
I would consider having a Document abstraction
which offers stuff for creating and applying edits
Bastien Jansen
@bjansen
Mar 17 2016 14:25
I see
Gavin King
@gavinking
Mar 17 2016 14:25
and everything else you need to work with a Document
why do we go to the ImportProposals to get Indents?
value indents => importProposals.indents;
that doesn’t seem right
Gavin King
@gavinking
Mar 17 2016 22:28
IDE build is broken now
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/typechecker/IdePhasedUnit.ceylon:64: error: actual member does not refine any inherited member: 'moduleSourceMapper' in 'IdePhasedUnit'
[ceylon-compile]     shared actual default BaseIdeModuleSourceMapper moduleSourceMapper => 
[ceylon-compile]                                                     ^
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/typechecker/IdePhasedUnit.ceylon:65: error: method or attribute does not exist: 'moduleSourceMapper' in type 'PhasedUnit'
[ceylon-compile]             unsafeCast<BaseIdeModuleSourceMapper>(super.moduleSourceMapper);
[ceylon-compile]                                                         ^
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/model/SourceFile.ceylon:20: error: cannot find symbol
[ceylon-compile]         extends CeylonUnit(phasedUnit.moduleSourceMapper) {
[ceylon-compile]                            ^
[ceylon-compile]   symbol:   method getModuleSourceMapper()
[ceylon-compile]   location: variable phasedUnit of type IdePhasedUnit
@davidfestal ?