myroslavarm / experimental-completion Goto Github PK

notEmpty
^ self isEmpty not

--> should use #notEmpty

If we add a method to the class side:

shouldInheritSelectors
^true

we then can override single tests in the subclass and it will run all tests (non-overriden inherited)

CodeCompletion: bug fix for visitBlockNode

see: pharo-project/pharo#4029

We check for a number of nodes that we are not intersted in in #createModel. We should add
the sequenceNode, too:

node isSequence ifTrue: [ ^NECEmptyModel new ].

when writing the selector at the start, what does the old show as possible completion?

It would be nice if we could add CompletionController already to the image and have people testing it.

it does not need much: CompletionController, CompletionContext, TypingVisitor what else?

The good thing would be that we could test it for real (even make it default for a while)

This could be added to the NECompletion package (in its own Tag, like OCompletion now)

there's now a better version of code completion available, with a single model class and a single entry class. it makes sense to implement it into Pharo 8, as it gives an even better idea of the end result of our code completion

The same way that we have CompletionController which defines that it uses CompletionContext
we could add the "we make our own model" version in a way that it is pluggable:

1. subclass CompletionContext (maybe CompletionContextWithOwnModel)
2. override there createModel to return our own model
3. add CompletionController with a method contextClass that returns CompletionContextWithOwnModel

--> the version of the new completion with our own model can be selected in the preferences and we can work on both at the same time

(I feat that the "own model" version will take a bit to get stable, while CompletionContext is very, very usable now)

in the new completion

This example:

self environment allBehaviors
select: [ :class | class slots anySa [ :slot | slot class == self ]].

when writing anySa... the old Code Completion suggests selectors. The new we get nothing.

This has something to do with the parse error node later, we need to explore how this is parsed.

for temporary variables specifically

when declaring temps, they get completed. they shouldn't because we are essentially making up new variables

there are some out of date comments that should be removed:

"to be implemented"

"self context: model."

the line
(self methodNames includes: aRBMethodNode) ifFalse: [ ^#() ].
in MatchedNodeProducer >> visitMethodNode: aRBMethodNode
gets rid of the incorrect temp definition completion, but also stops method definitions from being completed. we need to figure out a way to fix it

When I am in a class definition and start to add a new instance variable, I get a DNU

narrowWith: needs to be in a category

if we have arguments or variables in the definition of the method and if inside them method we pass them as arguments, they need to be suggested (first)

parseSource:

• a special doSemanticAnalysisIn: was added for RBProgramNode and the existing guarded for nil... but we could just instead use doSemanticAnalysis when the class is nil. Much simpler.

It feels a bit slow.

The reason: we #entries is called a lot. In the old Model it was an instance variable

we should refactor it to be that.

-> lazy init
-> update on narrowWith:

Something like that...

label is shown as 'todo', it should show the type if we know it instead

Ocompletion is about sorting results. We will add an API for that later.

For now everything would be simpler if we could remove the dedicated OCompletion code
(Controller and all the classes in the category)

There is a (right now disable) setting

Use navigation keys for extended completion functionality

if turned on, one gets with -> (right arrow) a preview of all the code for the possible methods.

==> turn it on and implement the missing methods.

in the old completion, we get a suggestion for the send.

With the new one, we do not as the parser parses all the faulty code in one parse error node.

First step: add two tests (one for old, one for now) so we do not forget.

for example when typing in 'init' the suggestions that appear don't include 'initialize' whereas it's actually the one most likely to be used and should appear at the top of the list

Right now it seems that we do not create a new AST for the suggestions in some cases, but instead we rely on updating the AST (see narrowWith: on the Node classes).

It might be better to instead update the AST by re-parsing it from source.

It is not really clear what happens in detail, so the first step is to understand the problem better

when pressing we get a DNU #commonPrefix

on a quick read, maybe the method from the old Model will just work if copied over

nodeForOffset: anInteger
| children |
children := self children.
(children isEmpty) ifTrue: [ (self sourceInterval includes: anInteger) ifTrue: [^self]].
children do: [:each | (each sourceInterval includes: anInteger) ifTrue: [^each nodeForOffset: anInteger] ].

testNodeForOffsetAssignment
"test the case of a Assignment"
| source ast foundNode |
source := 'method Object := Class'.
ast := RBParser parseMethod: source.
foundNode := ast nodeForOffset: 9.
self assert: (foundNode class == RBVariableNode).
foundNode := ast nodeForOffset: 14.
self assert: (foundNode class == RBAssignmentNode).
foundNode := ast nodeForOffset: 19.
self assert: (foundNode class == RBVariableNode)

testNodeForOffsetMessage
	"test the case of Messages"
	| source ast foundNode |
	source := 'method Object doit: Class'.
	ast := RBParser parseMethod: source.
	foundNode := ast nodeForOffset: 9.
	self assert: (foundNode class == RBVariableNode).
	foundNode := ast nodeForOffset: 14.
	self assert: (foundNode class == RBMessageNode).
	foundNode := ast nodeForOffset: 22.
	self assert: (foundNode class == RBVariableNode)
	
	
	testNodeForOffsetTempDefinition
		"test the case of Messages"
		| source ast foundNode |
		source := 'method | temp |'.
		ast := RBParser parseFaultyMethod: source.
		foundNode := ast nodeForOffset: 12.
		self assert: (foundNode class == RBVariableNode).

testNodeForOffsetVar
"test the case of a variable"
| source ast foundNode |
source := 'method Object'.
ast := RBParser parseMethod: source.
foundNode := ast nodeForOffset: 9.
self assert: (foundNode class == RBVariableNode)

The CompletionModel uses only one kind of entry from the old system: the NECGlobalEntry

We should instead use our own entry class

CodeCompletion: DNU when completing sends to Class Variables

There is a bug when completing sends to class variables
(e.g. take the ones in SmalltalkImage as an example).

Problem seems to be that the typer (class TestTypingVisitor) does not do the right thing
in visitGlobalNode:

issue tracker entry for Pharo is: pharo-project/pharo#4030

see this example:

ast := RBParser parseMethod: 'test
| te|'.

ast bestNodeFor: (10 to: 10)

Here we would expect the Temp node that is the temp definition to be returned

right now they get completed. we need to decide if it's good or not, and if not how to fix it

example:

test
| code tet |

-> code completion works.

but if one starts with

| tes

it does not work even if we put a | at the end.

we see a DNU when writing "self" --> Self node needs to forward to variable node?

If we return the right symbols when #type is called on the entry, the User Interface will color the suggestions (e.g. BOLD for classes).

see the implementors of #type in the old Entry hierarchy (e.g it should be #instVar for instance variables).

If the TestCompletionEntry is extended to store the node, it should be easy to implement this.

NEC still exists in Pharo 8 but there's no reason to keep it in this repo

When writing a comment "...." I get a red menu showing a DNU

• ivar currentClass is not used
• completionToken seems to be not used either

it's ok for globals for example, but when typing a method selector, it gives you a suggestion based on what you typed in one go, and further typing doesn't update the list

We can use the cache on Symbol:

^Symbol selectorTable

when adding a temp at the end of the | | part of a method --> red rendering error showing DNU

I wonder if using UndefinedObject is really good. e.g. this means we can not destinguish
"we look at UndefinedObject" vs. really having no class set (in the playground).

Can it not just be nil in the case there is no class?

Cyril got it in

needToAdaptTo: aRGClass
    1halt.
    ^ (self generatedTraits as (aRGClass traitComposition traits select: #isMetamodelEntity) and: [ (self generatedSlots collect: #name) = (aRGClass slots collect: #name) ]) not

trying to write self generatedTraits asSet

maybe can be fixed by creating

RBReturnNode >> completionToken
	^ Symbol selectorTable

because i don't think returning an empty string makes sense at this point because we do want a completion

CompletionModel needs a class comment