feat(auto-completion) #129

carmenberndt · 2020-05-08T13:24:20Z

closes #107

Context-aware auto-completion features:

Gets suggestions for available type, block, and field names and also supported functions and attributes.
Suggest semantically correct attributes / types (i.e. don't display attributes that would be invalid in this context)

…d relation attributes

…ttributes

carmenberndt · 2020-05-08T13:28:20Z

TODO

The auto-completion should also show inline docs (similar to JSDoc for Prisma Client JS)
write tests

divyenduz

Awesome work again, I think everything reported in the last review is fixed. We are getting closer and closer to this being super awesome 🎉

New observations:

There is only one model Post in the schema that I am using, is it intentional that it lists two options for it on cmd+click as if there are multiple model Posts? IMO unambiguous jumps should just happen, without the selection dialog

https://www.loom.com/share/735a7d62baf74c80bb7640ee5d9b87b9

Probably nitpick but this suggestion shouldn't come here

Probably a bug, cmd+clicking on an unambiguous User model also lists enum UserType in jump targets, I think this happens with models that start with another model name too

https://www.loom.com/share/a25242046ce24d9996f3d11c9e88bfbe

Co-authored-by: Tim Suchanek <Tim.Suchanek@gmail.com>

carmenberndt · 2020-05-19T12:12:38Z

There is only one model Post in the schema that I am using, is it intentional that it lists two options for it on cmd+click as if there are multiple model Posts? IMO unambiguous jumps should just happen, without the selection dialog

I can't reproduce this using this schema:

generator db {
  provider      = "prisma-client-js"
  binaryTargets = ["native", "windows"]
}

datasource db {
  provider = "postgresql"
  url      = env("AZURE_FUNCTIONS_WINDOWS_PG_URL")
}

enum UserType {
  CUSTOMER
  ADMIN
}

model User {
  email String   @unique
  name  String?
  posts  Post[]
  type  UserType

  @@id([email])
}

model Post {
  id        String   @default(cuid()) @id
  createdAt DateTime @default(now())
  updatedAt DateTime @updatedAt
  published Boolean  @default(true)
  title     String
  content   String?
  authorId  String?
  User      User?    @relation(fields: [userEmail], references: [email])
  userEmail String?
}

…code into autoCompletionResearch

carmenberndt · 2020-05-19T12:39:54Z

Probably a bug, cmd+clicking on an unambiguous User model also lists enum UserType in jump targets, I think this happens with models that start with another model name too

Should be fixed now

carmenberndt · 2020-05-22T07:49:41Z

Probably nitpick but this suggestion shouldn't come here

this is fixed now too 🙂

divyenduz

Awesome work again, looks really stable this time:

Nitpick - @@id when already used should not be in the suggestions

Nitpick - Bug - cmd + hover yields an error message, if the schema is invalid (note the @@ in the schema). This should probably just work (error tolerant parser) or be a no-op.

https://www.loom.com/share/478c186e7e044f56aab0b34217ae83df

Nitpick - Bug (or feature) - cmd + click on the model yields two jump to options, one for itself and one for the usage

https://www.loom.com/share/70a40f5443be41a0a51eea52ea7a6ec9

Bug - jump to type definition has two fields even though it is not ambiguous. I am aware that you were not able to reproduce this, here is a video of what I observe though

https://www.loom.com/share/051510147592480f9bb84eaf3ada64f8

Bug - starting a directive autocompletion with a wrong character forbids future autocompletions

https://www.loom.com/share/5529ccdd80fd4f2ebf71440e8bea55b2

Besides these, added some comments to review the code. There are some deep-copies (probably okay as none of them probably is on a variable array greater than a few items) but you can probably consider getting rid of the klona dependency altogether by using const array + map, filter reduce vs deep-copy + mutable variable. I repeat that doing this is not that important as the deep-copies are on small arrays.

divyenduz · 2020-05-25T09:04:05Z

server/src/MessageHandler.ts

  if (end < 0) {
    return ''
  }
  return currentLine.slice(beginning, end + position.character)
 }

-export async function handleDefinitionRequest(
+export class MyBlock {


Is the name MyBlock intentional? What does this represent?

When using the prismafile parser, there already as an object Block, but since I can't use this anymore, I called this MyBlock to not get confused with the other Object. Since I am not using the other dependency anmore, I guess I'll just rename this to Block again. This data structure is used to store all Blocks in a schema so I don't have parse every line in a string multiple times.

divyenduz · 2020-05-25T09:18:44Z

server/src/completions.ts

+      (item.includes('model') || item.includes('enum')) &&
+      item.includes('{')


This relies on Prisma fmt to work and being used. It won't support this variant:

model User {

We might want to use a parser for this one (prismafile?). There could be multiple reasons why a team don't use Prisma fmt (like conventions, failure to know that format on save is an option etc).

Prismafile is not able to parse error-tolerant, also this will soon be unsupported as well, since this functionality will be replaced soon for the SDK.

It's not supposed to work on this variant, because then we would need to define what's possible. E.g. allow writing the whole schema in one line, or having mulitple new lines in between the modelName and {, ... . Not sure if this is worth putting the time into. @janpio?

Difficult question. Auto completion should of course already work before formatting was applied, but if I read your comment right this would be a lot more complicated and complex right now @carmenberndt, correct?

yes, this would be quite easy to implement once we have a solution for the error-tolerant-parser. Then I can simply check where a model begins and ends and so on, right now this is quite complex since I am only working with strings here.

divyenduz · 2020-05-25T09:20:58Z

server/src/completions.ts

+function removeInvalidFieldSuggestions(
+  supportedFields: Array<string>,
+  block: MyBlock,
+  lines: Array<string>,
+  position: Position,
+): Array<string> {
+  let reachedStartLine = false
+  for (const [key, item] of lines.entries()) {
+    if (key === block.start.line + 1) {
+      reachedStartLine = true
+    }
+    if (!reachedStartLine || key === position.line) {
+      continue
+    }
+    if (key === block.end.line) {
+      break
+    }
+    const fieldName = item.replace(/ .*/, '')
+    if (supportedFields.includes(fieldName)) {
+      supportedFields.filter((field) => field !== fieldName)
+    }
+  }
+  return supportedFields
+}


Probably a code comment to specify what this functions does. I didn't understand this at all.

I added a comment.

divyenduz · 2020-05-25T09:22:49Z

server/src/completions.ts

+  position: Position,
+): CompletionItem[] {
+  // create deep copy
+  const suggestions: CompletionItem[] = klona(supportedDataSourceFields)


Is this deep-copy needed? The variable is not modified. I like this pattern of using filter over deep-copy + mutable variable like in other places.

The problem of using filter is it modifies the object, which leads to problems when using the object again later in a different context. Therefore this deep-copy is needed here.

divyenduz · 2020-05-25T09:23:29Z

server/src/completions.ts

+  position: Position,
+): CompletionItem[] {
+  // create deep copy
+  const suggestions: CompletionItem[] = klona(supportedGeneratorFields)


Is this deep-copy needed?

yes it is, since both .map() and .filter() operations change the data inside supportedGeneratorFields

carmenberndt · 2020-05-25T10:40:52Z

Nitpick - @@id when already used should not be in the suggestions

this is fixed now

carmenberndt · 2020-05-25T12:21:12Z

Nitpick - @@id when already used should not be in the suggestions

done

Nitpick - Bug - cmd + hover yields an error message, if the schema is invalid (note the @@ in the schema). This should probably just work (error tolerant parser) or be a no-op.

https://www.loom.com/share/478c186e7e044f56aab0b34217ae83df

can not reproduce this (both on windows and ubuntu)

Nitpick - Bug (or feature) - cmd + click on the model yields two jump to options, one for itself and one for the usage

https://www.loom.com/share/70a40f5443be41a0a51eea52ea7a6ec9

can not reproduce this (both on windows and ubuntu)

Bug - jump to type definition has two fields even though it is not ambiguous. I am aware that you were not able to reproduce this, here is a video of what I observe though

https://www.loom.com/share/051510147592480f9bb84eaf3ada64f8

can not reproduce this (both on windows and ubuntu)

Bug - starting a directive autocompletion with a wrong character forbids future autocompletions

https://www.loom.com/share/5529ccdd80fd4f2ebf71440e8bea55b2

done

janpio · 2020-05-25T13:50:54Z

language-configuration.json

@@ -22,5 +22,6 @@
    ["[", "]"],
    ["(", ")"],
    ["\"", "\""]
-  ]
+  ],
+  "wordPattern": "(-?\\d*\\.\\d\\w*)|([^\\`\\~\\!\\#\\%\\^\\&\\*\\(\\)\\-\\=\\+\\[\\{\\]\\}\\\\\\|\\;\\:\\'\\\"\\,\\.\\<\\>\\/\\?\\s]+)"


what a beautiful pattern 💩

I used the word pattern from here: https://code.visualstudio.com/api/language-extensions/language-configuration-guide#word-pattern and removed @

matthewmueller · 2020-05-25T15:15:03Z

Based on: #129 (review)

2. Nitpick - Bug - cmd + hover yields an error message, if the schema is invalid (note the @@ in the schema). This should probably just work (error tolerant parser) or be a no-op.

I can confirm this behavior as well on OSX. I'm holding alt while hovering over User.

https://www.loom.com/share/c3415ec6d19f4d1d9672585aef5f7fa5

3. Nitpick - Bug (or feature) - cmd + click on the model yields two jump to options, one for itself and one for the usage

I see 2 options too, but they're both the definition (not the caller). I'm not sure if this is expected or not (I don't use this peak feature).

4. Bug - jump to type definition has two fields even though it is not ambiguous. I am aware that you were not able to reproduce this, here is a video of what I observe though

@divyenduz do you have an example schema? I'm not able to reproduce this either.

janpio · 2020-05-25T17:23:02Z

Re 2) Can confirm this on windows, Ctrl + Hover while schema is invalid:

Re 3) Can confirm on Windows with valid schema:

(Also interpret it the way Matt does though)
This is already visible on Ctrl + hover only:

Re 4) This is actually 3 as well I think.

carmenberndt · 2020-05-26T07:55:30Z

I think the issue does not lie in the current branch. I could reproduce all of your issues only in case there is another Prisma extension installed at the same time. Re 2) this is a bug currently in Prisma Dev. Re 3) and Re 4) this happens because go to definition is defined slightly different here and in Prisma Dev/Prisma Extension. Therefore this suggests two options.

carmenberndt added 19 commits April 28, 2020 13:39

ifrst try at auto-completion

b278f6c

Merge branch 'master' into autoCompletionResearch

2949938

add suggestions for attributes and core types

50bb7b7

use prismafile parser to use position

f1cdfd2

trying out workaround for incomplete node parser

05d96ca

add suggenstion for block types

4cf0477

attribute suggestions depending on block

38c0a5d

suggestions for both core and relation types

0c41958

cleanup

c6ba9f0

code cleanup

ecb67de

fix check of field type

59e8044

field suggestion for @relation

d650941

use parser errors to create suggestions

ebbff1e

bump prismafile dependency

529921b

fix auto-completion for blocks for valid and invalid schemas

ec36646

fix auto-completion for field names for invalid and valid schema

bf9ffc1

fix auto-completion for types for valid and invalid schema

0cc8498

finish auto-completion for valid and invalid schemas for functions an…

22f7fe7

…d relation attributes

add auto-completion for valid and invalid schemas for functions and a…

0935936

…ttributes

carmenberndt changed the title ~~feat(Autocompletion)~~ feat(auto-completion) May 8, 2020

carmenberndt added this to the Beta 5 milestone May 8, 2020

carmenberndt added 2 commits May 8, 2020 16:57

show inline docs

0df345b

fix attirubtes autocompletion

9c7b1c8

carmenberndt marked this pull request as ready for review May 8, 2020 15:14

carmenberndt requested review from matthewmueller, divyenduz and janpio May 8, 2020 15:15

carmenberndt added 2 commits May 11, 2020 14:11

remove prismafile dependency as it fails with some schemas

dd6d09b

fix integration tests

525032a

divyenduz suggested changes May 19, 2020

View reviewed changes

Update server/src/completions.ts

37f8148

Co-authored-by: Tim Suchanek <Tim.Suchanek@gmail.com>

carmenberndt added 2 commits May 19, 2020 14:39

fix bug jumping to wrong model/enum

813434c

Merge branch 'autoCompletionResearch' of https://github.com/prisma/vs…

12a3705

…code into autoCompletionResearch

carmenberndt added 2 commits May 22, 2020 09:24

merge master

13a0cf0

fix bug showing attribute suggestion in wrong position

dc03c8e

carmenberndt requested a review from divyenduz May 22, 2020 07:50

carmenberndt mentioned this pull request May 22, 2020

feat(): doc comments on hover #151

Merged

carmenberndt added 3 commits May 22, 2020 16:51

merge master

a2d83eb

adjust e2e tests to new prisma-fmt

5828c51

merge master

7393657

divyenduz suggested changes May 25, 2020

View reviewed changes

carmenberndt added 2 commits May 25, 2020 11:44

code cleanup

5e862c7

add comment

d2f9ec4

adjust block attribute suggestions

cf9c0b4

add word pattern to recognize @ as special character

e1a38f3

janpio reviewed May 25, 2020

View reviewed changes

carmenberndt mentioned this pull request May 26, 2020

Message: Request textDocument/definition failed with message: Schema parsing #144

Closed

carmenberndt added 2 commits May 26, 2020 10:20

merge master

0d3f4e2

merge master

f474c72

carmenberndt merged commit 4d6dbfd into master May 26, 2020

carmenberndt deleted the autoCompletionResearch branch May 26, 2020 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auto-completion) #129

feat(auto-completion) #129

carmenberndt commented May 8, 2020 •

edited

carmenberndt commented May 8, 2020 •

edited

divyenduz left a comment

carmenberndt commented May 19, 2020

carmenberndt commented May 19, 2020

carmenberndt commented May 22, 2020 •

edited

divyenduz left a comment

divyenduz May 25, 2020

carmenberndt May 25, 2020

divyenduz May 25, 2020

carmenberndt May 25, 2020

janpio May 25, 2020

carmenberndt May 25, 2020

divyenduz May 25, 2020

carmenberndt May 25, 2020

divyenduz May 25, 2020

carmenberndt May 25, 2020 •

edited

divyenduz May 25, 2020

carmenberndt May 25, 2020

carmenberndt commented May 25, 2020

carmenberndt commented May 25, 2020 •

edited

janpio May 25, 2020

carmenberndt May 25, 2020

matthewmueller commented May 25, 2020 •

edited

janpio commented May 25, 2020 •

edited

carmenberndt commented May 26, 2020 •

edited

		(item.includes('model') \|\| item.includes('enum')) &&
		item.includes('{')

feat(auto-completion) #129

feat(auto-completion) #129

Conversation

carmenberndt commented May 8, 2020 • edited

carmenberndt commented May 8, 2020 • edited

divyenduz left a comment

Choose a reason for hiding this comment

carmenberndt commented May 19, 2020

carmenberndt commented May 19, 2020

carmenberndt commented May 22, 2020 • edited

divyenduz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carmenberndt May 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carmenberndt commented May 25, 2020

carmenberndt commented May 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewmueller commented May 25, 2020 • edited

janpio commented May 25, 2020 • edited

carmenberndt commented May 26, 2020 • edited

carmenberndt commented May 8, 2020 •

edited

carmenberndt commented May 8, 2020 •

edited

carmenberndt commented May 22, 2020 •

edited

carmenberndt May 25, 2020 •

edited

carmenberndt commented May 25, 2020 •

edited

matthewmueller commented May 25, 2020 •

edited

janpio commented May 25, 2020 •

edited

carmenberndt commented May 26, 2020 •

edited