Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SourceKit] docs for complete.open #5645

Merged
merged 5 commits into from Nov 14, 2016

Conversation

Aghassi
Copy link
Contributor

@Aghassi Aghassi commented Nov 5, 2016

  • Contains documentation to Protocol.md for complete.open. I wasn't sure how to format these "sub" commands so to speak, so I did my best. I expect it is going to need some revisions.

Resolves SR-2117.

@Aghassi Aghassi changed the title Sourcekit docs complete.open [SourceKit] docs for complete.open Nov 5, 2016
@harlanhaskins
Copy link
Collaborator

@benlangmuir Mind reviewing?

Copy link

@thomasbarwanitz thomasbarwanitz left a comment

Choose a reason for hiding this comment

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

Nice

Copy link
Member

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @Aghassi! I have some suggestions inline.

@@ -54,6 +54,10 @@ provided.
}
```

| Request Name | Request Key | Description |
| -------------:|:------------|:------------|
| `open` | `complete.open` | Given an input file, will determine at each offset what keyword is possible. Default offset is `0` if none is provided. |
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we move this table up above the codecomplete request description, and add both the original codecomplete request and the newer codecomplete.open, codecomplete.close, and codecomplete.update requests. If you only want to cover codecomplete.open in this PR, that's fine, but we should at least add codecomplete as well.

Please note, the request key is codecomplete.open, not just complete.open. For the description, I suggest something like "Open a code-completion session for the given input file and offset, and return the initial list of completions". The key difference between codecomplete.open and codecomplete is that the former opens a session that can filter completions using codecomplete.update and must be closed with codecomplete.close, whereas the latter just returns all of the results without filtering.

We should also describe what else goes in the request, similar to the description of the codecomplete request, or at least describe the differences between that request and this one (I think the only difference in the request structure is that codecomplete.open can take an options dictionary).

<key.typename>: (string) // Text describing the type of the result.
<key.context>: (UID) // Semantic context of the code completion result.
<key.num_bytes_to_erase>: (int64) // Number of bytes to the left of the cursor that should be erased before inserting this completion result.
<key.substructure>: (dictionary) // Can contains an array the `nameoffset`, `namelength`, `bodyoffset`, `bodylength`, and `substructure`. Other `substructure` will exist if the code complete represents a function. Each one is a paramater of the function
Copy link
Member

Choose a reason for hiding this comment

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

I would say, "Contains an array of dictionaries representing ranges of structural elements in the result description, such as the parameters of a function". You don't need to list the keys here since you have them below.

@Aghassi
Copy link
Contributor Author

Aghassi commented Nov 8, 2016

@benlangmuir I have pushed a change reflecting your comments. I would like to work and get these initial formatting changes in first before expanding to the other commands. However, I am happy to work on those in a separate PR once this one is done so the foundation is there in case someone has more time to commit to this than I do.

Some of the wording in this request is lacking because I'm not 100% on what needs to be said, so I didn't want to say the wrong thing.

[opt] <key.sourcetext>: (string) // Source contents.
[opt] <key.sourcefile>: (string) // Absolute path to the file.
<key.offset>: (int64) // Byte offset of code-completion point inside the source contents.
[opt] <key.options>: (dict) // An options dictionary containing keys.
Copy link
Member

Choose a reason for hiding this comment

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

key.codecomplete.options

@@ -38,6 +38,11 @@ must be given either the path to a file (`key.sourcefile`), or some text
(`key.sourcetext`). `key.sourcefile` is ignored when `key.sourcetext` is also
provided.

| Request Name | Request Key | Description |
| -------------:|:------------|:------------|
| `codecomplete` | `codecomplete` | Given a file will open a code-completion session which can be filtered upon using `codecomplete.update`. Each session must be closed using `codecomplete.close`.|
Copy link
Member

Choose a reason for hiding this comment

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

This is a description of codecomplete.open. codecomplete just returns completions and does not filter or create a session. Also, you could remove the word "upon" entirely.

-  key.codecomplete.options, not key.options
-  Moved codecomplete definition to codecomplete.open
and vice versa
Copy link
Member

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

One last tweak and I think this will be good to go.

@@ -40,8 +40,8 @@ provided.

| Request Name | Request Key | Description |
| -------------:|:------------|:------------|
| `codecomplete` | `codecomplete` | Given a file will open a code-completion session which can be filtered upon using `codecomplete.update`. Each session must be closed using `codecomplete.close`.|
| `open` | `codecomplete.open` | Open a code-completion session for the given input file and offset, and return the initial list of completions. |
| `codecomplete` | `codecomplete` | Open a code-completion session for the given input file and offset, and return the initial list of completions. |
Copy link
Member

Choose a reason for hiding this comment

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

codecomplete does not open a session. It just returns completions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, to clarify, codecomplete would be defined as "Returns completions given an input file and offeset". Does that mean (for my own understanding) it opens a session internally and closes it?

Copy link
Member

Choose a reason for hiding this comment

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

Abstractly you could sort of think of it that way, but it doesn't really work that way. The "session" for the requests open/close/update is really a way of storing the results we get back from code-completion in the compiler and manipulating them. Code-complete just returns the results and is done.

@benlangmuir
Copy link
Member

@swift-ci please smoke test and merge

@benlangmuir
Copy link
Member

Thanks for working on this!

@tkremenek
Copy link
Member

@swift-ci smoke Test and merge

@swift-ci swift-ci merged commit beaf946 into apple:master Nov 14, 2016
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.

None yet

6 participants