You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/06/03 18:58:29 UTC

[GitHub] [daffodil-vscode] Shanedell opened a new issue, #165: Autocompletion remove wholeLine checks

Shanedell opened a new issue, #165:
URL: https://github.com/apache/daffodil-vscode/issues/165

   I still don't understand these `wholeLine.includes` checks. The context to use for autocompletion doesn't have do with the line of text--it has to do with things like what is the name of the current tag that the cursor is inside, what are the values of it's attributes, etc. This just feels very fragile and likely to give the wrong results.
   
   It seems like instead you want something like
   ```
   if (nearestOpenItem.includes("element)) {
     if (nearesOpenItem.hasAttribute("name") || nearestOpenItem.hasAttribute("ref")) {
        ...
     }
   }
   ```
   
   A simple examle where this breaks, it's not uncommon for schemas to use "xsd" instead of "xs" (or even no prefix). In those cases looking for "xs:element" is going to fail.
   
   I wont' block this merging for this, but hope it's updated in the future to improve how it determines the appropriate context for choosing completions.
   
   _Originally posted by @stevedlawrence in https://github.com/apache/daffodil-vscode/pull/117#discussion_r889129269_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243775104

   In your explanation above is a closing tag a tag with either "</" or "/>" or do just mean '>'?  I think of
   
   > <sequence> as an opening tag and
   
   > </sequence> or 
   
   > <sequence someattribute=somevalue /> as a closing tags.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] stevedlawrence commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

stevedlawrence commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1195808994

   Note: I'm unassigning myself since I'm not actually working on this issue. I'd prefer we use @ if you need to call someone's attention to an issue.
   
   > Based on VS Code examples for autocompletion, the string method "contains()" was used to return true or false to a string comparison of possible XML components that were in the line where the autocomplete was triggered.
   
   This is the crux of the issue. For example, I can easily imagine an example like this:
   
   ```xml
   <xs:element name="foo"
      dfdl:someProperty="bar"
      |
   ```
   Where we are autocompleting on the last line of this where the pipe character is. The context of what is avialbale to autocomplete has nothing to do with the line where the autocomplete was triggered. You need to inspect what type of node were are actually in (i.e. element, group, sequence, etc.), which is why I suggested something like `openItem` to figure out what the openItem is.
   
   > One relatively easy way to provide an alternative to the hardcoded namespace "xs" could be to use the JavaScript/TypeScript object "regExp" to define a regex that compares '<' + any characters + "element"
   
   Ideally one would inspect the current XML namespace scope to determine what prefixes are mapped to to the XMLSchema namespace and then use those prefixes determine the element. That's extra complication though, I think the hardcoded prefix issue should be resolved as another issue, I was just showing one example where this code might fail. Assuming the namespace prefix is `xs` is probably reasonable for now, thought we are starting to move away from that.  
   
   > The entire document could also be parsed into XML components and associated with positions and some method developed to query if a tag is open by position, although thought would have to be given to how to handle inserting attributes into already closed tags of existing schemas. 
   
   I agree that this adds extra difficulty, and probably isn't the best solution especially from a performance perspective. We don't want to have to parse the entire document everytime an character is inserted or we autocomplete--this will be impossibly slow with large schemas. But this is sort of the right idea I think. Rather than just looking at the first line, we look back far enough to get a reasonable idea about what the context is. In some cases that context will be on the same line, but often times it won't be. The `openItem` function seems to do something like that. I think that would be a happy medium of performance and accuracy of autocompletion context.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243769417

   In your explanation above is a closing tag a tag with either "</" or "/>" or do just mean '>'?  I think of
   "<sequence>" as an opening tag and
   "</sequence>" or "<sequence someattribute=somevalue />" as a closing tags.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] Shanedell commented on issue #165: Autocompletion remove wholeLine checks

Posted by "Shanedell (via GitHub)" <gi...@apache.org>.

Shanedell commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1507697832

   Through multiple PRs the requirements for this Issue have a been met.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243733401

   I may be misunderstanding what you mean by auto complete a tag. It seems like you are saying the completion tag should not be created at the time a beginning tag is ended with a '>'.  If that is the case how would you handle editing an existing schema to add a attribute to an item where the end tag for everything already exists? In that case it would seem you have to look forward and backward, but since you can't know if the change is an addition to existing code or to new code, you would always have to look forward and backward and keep track of all the opening and closing tags.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1192798296

   VS Code has a built in interface for handling auto-complete. Whenever an autocomplete trigger is activated, the built in interface supplies a position in the document where the autocomplete was triggered.  With the position there is a method to get the "lineAt()" string.  The wholeLine variable contains the string returned by lineAt(). A more accurate variable name instead of "wholeLine" might have been "triggerLine".  Based on VS Code examples for autocompletion, the string method "contains()" was used to return true or false to a string comparison of possible XML components that were in the line where the autocomplete was triggered.  In the current autocomplete code no attempt is made to parse out individual XML components, string comparisons are the basis for determining what autocompletion items should be suggested.  An "openItem" is determined by parsing lines backwards to find the last end tag. Any XML components after that last end tag are considered to be "open". One relat
 ively easy way to provide an alternative to the hardcoded namespace "xs" could be to use the JavaScript/TypeScript object "regExp" to define a regex that compares '<' + any characters + "element", and uses the regExp test() method to compare the regex to wholeLine and return true of false, similar to how the process currently works.  Another suggestion was to use regex to parse the namespace from wholeLine and then use the namespace value to build an end tag value to look for, I'm not sure how the search for the end tag would work.  The entire document could also be parsed into XML components and associated with positions and some method developed to query if a tag is open by position, although thought would have to be given to how to handle inserting attributes into already closed tags of existing schemas. There are probably any number of solutions. Each solution will have benefits and drawbacks, including length of development time. I'm not sure the best way to address this issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] Shanedell closed issue #165: Autocompletion remove wholeLine checks

Posted by "Shanedell (via GitHub)" <gi...@apache.org>.

Shanedell closed issue #165: Autocompletion remove wholeLine checks
URL: https://github.com/apache/daffodil-vscode/issues/165


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] stevedlawrence commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

stevedlawrence commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243796382

   By closing tag I just mean `</foo>`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] stevedlawrence commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

stevedlawrence commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243750168

   By a tag, I just mean autocomplete should suggest `xs:element`, `xs:sequence`, etc. XML calls these all "elements", but that's a bit confusing since "element" can have a different meaning in XML Schema, so I just used tag, which I think is a sortof unofficial name for an XML element.
   
   I'm fine if you want to create the closing tag when you also create the opening tag.
   
   > how would you handle editing an existing schema to add a attribute to an item where the end tag for everything already exists?
   
   I don't think I understand this question. Maybe an example of when yo mean would help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243764098

   In your explanation above is a closing tag a tag with either "</" or "/>" or do just mean '>'?  I think of


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rthomas320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by "rthomas320 (via GitHub)" <gi...@apache.org>.

rthomas320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1488587179

   The code is more flexible and more easily updated, adding or modifying auto-completion items has been simplified and streamlined. The specific testcases Steve described are handles by the new code, so I think there is a good chance that it should be closed. But we should get @stevedlawrence 's opinion.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] stevedlawrence commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

stevedlawrence commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1238140294

> initial value of whatever text was on the editor line that triggered the autosense

This is what I don't really understand. The contents of the current line is not the context to use to figure out what to use for completion. For example, in theory you could have an entire schema on a single line with no line breaks. In which case the wholeLine checks would be meaningless. What really matters is context immediately around the cursor when the completion is triggered, which requires look behind the cursor to find the current tag.

Your code works, but I think it's because you're doing the right checks **in addition** to the wrong wholeLine checks. For example, to complete the attributes for a sequence, you have this:

```js
if (
checkLastItemOpen(document, position) &&
(wholeLine.includes('<xs:sequence') ||
checkSequenceOpen(document, position))
) {
return getCompletionItems(
[
'dfdl:hiddenGroupRef=',
'dfdl:sequenceKind=',
'dfdl:separator=',
'dfdl:separatorPosition=',
'dfdl:separatorSuppressionPolicy',
],
preVal
)
}
}
```

Whether or not the `wholeLine.includes("<xs:seqeuence")` has nothing to do with whether you need to provide those completion items. All that matters if you are inside a sequence tag. I *think* your `checkLastItemOpen` and `checkSequenceOpen` checks are what do this and why your examples work, though I haven't dug into exactly what those checks do. Seems these `wholeLine` checks should be unnecessary.

Note that you don't even really care if a sequence is open. We only care if we are in a sequence tag. For example, if you have this:

```xml
<xs:sequence |>
...
</xs:sequence>
```

And autocomplete where the pipe is, the sequence isn't really open, but we are inside a sequence tag. Maybe the checkSequenceOpen takes this into account and considers it open if it's inside it?

Note that your completion logic for choice only checks `wholeLine` and doesn't have any checkChoiceOpen or anything. Interestingly, I can't get choices to autocomplete for me. I'm not sure why it works in your gif, but it might be related?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243771288

   In your explanation above is a closing tag a tag with either "</" or "/>" or do just mean '>'?  I think of
   "<sequence>" as an opening tag and
   "</sequence>" or "<sequence someattribute=somevalue />" as a closing tags.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] Shanedell commented on issue #165: Autocompletion remove wholeLine checks

Posted by "Shanedell (via GitHub)" <gi...@apache.org>.

Shanedell commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1478102724

   @stevedlawrence Would you be able to take a look over the Intellisense and comment here if you believe this issue is resolved or still needs some addressing?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1243785893

   In your explanation above is a closing tag a tag with either "</" or "/>" or do just mean '>'?   I think of
   "\<xs:sequence\>" as an opening tag and
   "\</xs:sequence\>" as a closing tag
   "\<sequence someattribute=somevalue /\>" as an opening tag ('<') and a closing tag "(/>)".
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] stevedlawrence commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

stevedlawrence commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1240872286

To clarify what I think the correct algorithm is determine the right context for auto-completion:

1. From the current position, scan backwards until until you find either a `<` or `>`
1. If we find a `<`, it means we are inside an open tag (i.e. adding/modifying an attribute)
1. From the `<` character, read characters forward until we find a space (or the current position), this is the context tag
1. If we read read no characters, it means we just have a `<`, and should jump to `ii`.
2. Otherwise, continue to scan forward keeping track if we are in an attribute name, or value, or neither. Once we reach the current position, we will know if we are inside an attribute value or not
1. If we are in an attribute value, we can autocomplete the possible values for the attribute we found
2. If we are not in an attribute value, we can autocomplete the possible attributes for the tag we found
2. If we find a `>`, it means we should autocomplete a new tag
1. Scan backwards to find the parent element. This is slightly tricky, since we need to ignore previous sibling elements, which could be deeply nested. I think the algorithm is increment a count when we see a closing tag, decrement a count when we see an opening tag. If we find an opening tag and that count is zero, then we've found the parent tag.
2. Once we've determined the parent tag, we can autocomplete which tags are allowed as a child of that tag (e.g.. xs:element cannot have an xs:sequence as a child)

Note that none of this looks at entire lines. It looks character by character, either forwards or backwards, to determine the correct context of the current position.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] rt320 commented on issue #165: Autocompletion remove wholeLine checks

Posted by GitBox <gi...@apache.org>.

rt320 commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1236132433

   @stevedlawrence I attached a couple of animated GIFs to demonstrate what happens in the current auto completion for your example, and also a slightly more complex example.  
   ![ElementEndTest3](https://user-images.githubusercontent.com/98881959/188274741-1e40dacd-6d7f-409d-9639-95c8c242ec13.gif)
   I am planning to make some changes to the code, but keep the overall logic.  As a minimum I will rename the wholeline variable to suggestionTrigger as it will contain as the initial value of whatever text was on the editor line that triggered the autosense completion function. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [daffodil-vscode] Shanedell commented on issue #165: Autocompletion remove wholeLine checks

Posted by "Shanedell (via GitHub)" <gi...@apache.org>.

Shanedell commented on issue #165:
URL: https://github.com/apache/daffodil-vscode/issues/165#issuecomment-1472698901

   @rt320 Should this be closed, did your most recent big PR for Intellisense fix this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org