Skip to content

Conversation

@nicolasstucki
Copy link
Contributor

No description provided.

.drone.yml Outdated
group: test
image: lampepfl/dotty:2017-11-17
commands:
- cp -R . /tmp/3/ && cd /tmp/3/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a unique directory

(fnd, exp, totalChange.toDouble / (expected.length + found.length))
}

def mkColoredLineDiff(expected: Seq[String], actual: Seq[String]): String = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some documentation?


class CompletionTest {

@Test def competion0: Unit = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: competion -> completion


import dotty.tools.languageserver.util.embedded._

object Code {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file, and every other file in this directory would benefit from some documentation.


ai.next() match {
case emb: CodeMarker =>
positions += Tuple3(emb, line, char)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use += ((a, b, c))

var char = 0
def scan(str: String): Unit = {
for (c <- str)
if (c == '\n') { line += 1; char = 0 } else { char += 1 }
Copy link
Member

@smarter smarter Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just line = str.count(_ == '\n'); char = str.length - line ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly, char needs to be set to 0 when we encounter \n


override def onMarker(marker: CodeMarker): Exec[Unit] = {
val res = server.definition(fix(marker.toTextDocumentPositionParams)).get()
assert(res.size() == refOpt.size, res)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size() -> size

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

res is a java collection, hence it requires size().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was complaining or waring at the time. I will double check.

import dotty.tools.languageserver.util._
import dotty.tools.languageserver.util.embedded.CodeMarker

class CodeDefinition(val range: CodeRange, refOpt: Option[CodeRange]) extends ActionOnRange {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can return more than one definition.


def to(other: CodeMarker): CodeRange = CodeRange(this, other)

def file: PosCtx[TestFile] = implicitly[PositionContext].positionOf(this)._1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add implicit def posCtx(implicit ctx: PositionContext): PositionContext = ctx in the class to be able to do posCtx.positionOf ...

| "compilerVersion" : "0.6.0-bin-SNAPSHOT-nonbootstrapped",
| "compilerArguments" : [ "-feature", "-deprecation", "-unchecked", "-Xfatal-warnings", "-encoding", "UTF8", "-language:existentials,higherKinds,implicitConversions" ],
| "sourceDirectories" : [ "$baseDir/out/ide-tests/src" ],
| "dependencyClasspath" : [ "$baseDir/library/../out/bootstrap/dotty-library-bootstrapped/scala-0.7/classes", "$ivyDir/cache/org.scala-lang/scala-library/jars/scala-library-2.12.4.jar" ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded version number should be fixed before this is merged.

import dotty.tools.languageserver.util.PositionContext._
import org.eclipse.lsp4j._

class SymInfo(name: String, kind: String, range: CodeRange, container: String) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use SymbolKind for kind instead of String ?

@smarter
Copy link
Member

smarter commented Feb 22, 2018 via email

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

val result = server.completion(marker.toTextDocumentPositionParams).get()
assert(result.isRight, result)
assert(!result.getRight.isIncomplete, s"Completion results were 'incomplete': $result")
assertTrue(s"Completion results where not 'right': $result", result.isRight)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: where -> where

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> were

*
* @see dotty.tools.languageserver.util.actions.CodeHover
*/
def hover(range: CodeRange, expected: String): CodeTester =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method and the following ones could return this.type instead of CodeTester, that way you don't have to document @return, it's obvious.

@Duhemm
Copy link
Contributor

Duhemm commented Apr 12, 2018

@nicolasstucki Do you want to add something?

Copy link
Contributor Author

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes from @Duhemm LGTM

@allanrenucci
Copy link
Contributor

@Duhemm Can you please merge master into topic/sbt1 if this happens to be merged before sbt1?

@allanrenucci allanrenucci removed their request for review April 12, 2018 11:05
nicolasstucki and others added 19 commits April 12, 2018 18:55
While working on the IDE tests, I noticed that the Dotty IDE didn't
behave as I expected in the following scenario:

  // Rename `Foo` to `Bar`
  class Foo { new Foo }

The Dotty IDE would consider that 3 places need changing, instead of
only 2:

 - The `Ident(Foo)` in the `TypeDef`
 - The `Ident(Foo)` in the constructor
 - The `Ident(Foo)` in `Select(new Foo, <init>)`

The third tree, however is synthetic: it doesn't need to be changed in
the editor.

Trees whose positions are not derived from the source are now excluded
when doing a rename.
@Duhemm Duhemm merged commit 0ba6d2e into scala:master Apr 12, 2018
@Duhemm Duhemm deleted the ide-tests branch April 12, 2018 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants