stevedlawrence commented on code in PR #309:
URL: https://github.com/apache/daffodil-vscode/pull/309#discussion_r982690833


##########
src/language/providers/attributeCompletion.ts:
##########
@@ -71,19 +73,19 @@ export function getAttributeCompletionProvider() {
         ) {
           if (nearestOpenItem.includes('element')) {
             var preVal = ''
-            if (!wholeLine.includes('xs:element')) {
+            if (!wholeLine.includes(nsPrefix + 'element')) {

Review Comment:
   Did we just miss existing `xs:` removals that should have been included in 
PR #294? Is this intended fix that?
   
   From what I can tell, this does not address #165. 



##########
src/language/providers/closeElement.ts:
##########
@@ -32,93 +37,188 @@ export function getCloseElementProvider() {
         const wholeLine = document
           .lineAt(position)
           .text.substr(0, position.character)
+        var itemsOnLine = getItemsOnLineCount(wholeLine)
 
         if (
-          !wholeLine.includes('</') &&
           wholeLine.endsWith('>') &&
-          (wholeLine.includes('<' + nsPrefix + 'element') ||
-            nearestOpenItem.includes('element') ||
-            wholeLine.includes('<' + nsPrefix + 'group') ||
-            nearestOpenItem.includes('group') ||
-            wholeLine.includes('<' + nsPrefix + 'sequence') ||
-            nearestOpenItem.includes('sequence') ||
-            wholeLine.includes('<' + nsPrefix + 'simpleType') ||
-            nearestOpenItem.includes('simpleType') ||
-            wholeLine.includes('<' + nsPrefix + 'choice') ||
-            nearestOpenItem.includes('choice') ||
-            wholeLine.includes('dfdl:defineVariable') ||
-            nearestOpenItem.includes('Variable'))
+          ((wholeLine.includes('<' + nsPrefix + 'element') &&
+            nearestOpenItem.includes('element')) ||
+            (wholeLine.includes('<' + nsPrefix + 'group') &&
+              nearestOpenItem.includes('group')) ||
+            (wholeLine.includes('<' + nsPrefix + 'sequence') &&
+              nearestOpenItem.includes('sequence')) ||
+            (wholeLine.includes('<' + nsPrefix + 'simpleType') &&
+              nearestOpenItem.includes('simpleType')) ||
+            (wholeLine.includes('<' + nsPrefix + 'choice') &&
+              nearestOpenItem.includes('choice')) ||
+            (wholeLine.includes('dfdl:defineVariable') &&
+              nearestOpenItem.includes('Variable')))
         ) {
           var range = new vscode.Range(backpos, position)
           vscode.window.activeTextEditor?.edit((editBuilder) => {
             editBuilder.replace(range, '')
           })
-          if (
-            wholeLine.endsWith('>') &&
-            (wholeLine.includes('<' + nsPrefix + 'element ref') ||
-              wholeLine.includes('<' + nsPrefix + 'group ref'))
-          ) {
-            insertSnippet(' />\n$0', backpos)
-          } else if (
-            wholeLine.endsWith('>') &&
-            (wholeLine.includes('<' + nsPrefix + 'element') ||
-              nearestOpenItem.includes('element'))
-          ) {
-            insertSnippet('>\n\t$0\n</' + nsPrefix + 'element>', backpos)
-          } else if (
-            wholeLine.endsWith('>') &&
-            (wholeLine.includes('<' + nsPrefix + 'group') ||
-              nearestOpenItem.includes('group'))
-          ) {
-            insertSnippet('>\n\t$0\n</' + nsPrefix + 'group>', backpos)
-          } else if (
-            (wholeLine.endsWith('>') &&
-              wholeLine.includes('<' + nsPrefix + 'sequence')) ||
-            nearestOpenItem.includes('sequence')
-          ) {
-            insertSnippet('>\n\t$0\n</' + nsPrefix + 'sequence>', backpos)
-          } else if (
-            (wholeLine.endsWith('>') &&
-              wholeLine.includes('<' + nsPrefix + 'choice')) ||
-            nearestOpenItem.includes('choice')
-          ) {
-            insertSnippet('>\n\t$0\n</' + nsPrefix + 'choice>', backpos)
-          } else if (
-            (wholeLine.endsWith('>') &&
-              wholeLine.includes('<' + nsPrefix + 'simpleType')) ||
-            nearestOpenItem.includes('simpleType')
-          ) {
-            insertSnippet('>\n\t$0\n</' + nsPrefix + 'simpleType>', backpos)
-          } else if (
-            (wholeLine.endsWith('>') &&
-              wholeLine.includes('dfdl:defineVariable')) ||
-            nearestOpenItem.includes('defineVariable')
-          ) {
-            var startPos = document.lineAt(position).text.indexOf('<', 0)
-            var range = new vscode.Range(backpos, position)
-            vscode.window.activeTextEditor?.edit((editBuilder) => {
-              editBuilder.replace(range, '')
-            })
-            insertSnippet('>\n</dfdl:defineVariable>\n', backpos)
-            var backpos2 = position.with(position.line + 2, startPos - 2)
-            insertSnippet('</<' + nsPrefix + 'appinfo>\n', backpos2)
-            var backpos3 = position.with(position.line + 3, startPos - 4)
-            insertSnippet('</<' + nsPrefix + 'annotation>$0', backpos3)
-          } else if (
-            (wholeLine.endsWith('>') &&
-              wholeLine.includes('dfdl:setVariable')) ||
-            nearestOpenItem.includes('setVariable')
-          ) {
-            var startPos = document.lineAt(position).text.indexOf('<', 0)
-            var range = new vscode.Range(backpos, position)
-            vscode.window.activeTextEditor?.edit((editBuilder) => {
-              editBuilder.replace(range, '')
-            })
-            insertSnippet('>\n</dfdl:setVariable>\n', backpos)
-            var backpos2 = position.with(position.line + 2, startPos - 2)
-            insertSnippet('</' + nsPrefix + 'appinfo>\n', backpos2)
-            var backpos3 = position.with(position.line + 3, startPos - 4)
-            insertSnippet('</' + nsPrefix + 'annotation>$0', backpos3)
+          if (!wholeLine.includes('</') && itemsOnLine == 1) {
+            if (
+              wholeLine.endsWith('>') &&
+              (wholeLine.includes('<' + nsPrefix + 'element ref') ||
+                wholeLine.includes('<' + nsPrefix + 'group ref'))
+            ) {
+              insertSnippet(' />\n$0', backpos)
+            } else if (
+              wholeLine.endsWith('>') &&
+              wholeLine.includes('<' + nsPrefix + 'element') &&

Review Comment:
   wholeLine checks are not the way to solve #165 if that is the intention with 
this change.
   
   I have provided a rough algorithm of one way this could be solved. I'm sure 
there are other alternatives, but anything that does `wholeLine.endsWith` or 
`wholeLine.excludes` will not solve 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to