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/09/14 17:56:13 UTC
[GitHub] [daffodil-vscode] rt320 opened a new pull request, #294: Resolves Remove Prefixes Issue #120
rt320 opened a new pull request, #294:
URL: https://github.com/apache/daffodil-vscode/pull/294
-add function to parse prefix from schema tag and return the prefix plus a colon or return nothing -added calls to the new function
-modified the intellisense snippets to prepend the prefix value to the auto complete return element closes issue #120
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973028615
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
I'm not sure if we need this check if a line includes `schema`. We can just run the regex and it will return null if there's no match, which you already do.
##########
src/language/providers/utils.ts:
##########
@@ -17,6 +17,8 @@
import * as vscode from 'vscode'
+const schemaPrefixRegEx = new RegExp('<(.*?)schema')
Review Comment:
This is maybe a bit too broad of a regex? For example, it would match this:
```xml
<xs:import schemaLocation="..." />
```
And the capture group would be `xs:import `.
The regex would also match this:
```xml
</xs:schema>
```
And the capture group would be `/xs:` with the `/` included.
So an alternative regex would be:
```js
new RegExp('</?(|[^ ]+:)schema')
```
This ensures the only thing that can match between `<` and `schema` is either the empty string or one or more non-space characters followed by a colon. It also makes the leading slash optional.
It's a bit more complex (and it feels like this is some toy regex to use as many special characters as possible :smile:), but it is probably the most accurate regex to avoid false positives, especially if this ends up getting using in more places as the extension gets bigger and more complex.
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
+ let text = lineText.match(schemaPrefixRegEx)
+ if (text != null) {
+ console.log(text[1])
Review Comment:
Remove this log.
--
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 pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1256393714
Also, ignoring the spaces in package.json this also added a `__metadata` item, which I think we determined in another PR should not be there?
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972910319
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
Review Comment:
document.lineAt() returns a TextLine. The includes() property isn't available on a TextLine, It is available on a string, which is what is returned by the proprty text.substr().
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973128622
##########
src/language/providers/utils.ts:
##########
@@ -17,6 +17,8 @@
import * as vscode from 'vscode'
+const schemaPrefixRegEx = new RegExp('<(.*?)schema')
Review Comment:
I'll make the change and test it
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973145818
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
With the if statement removed autocomplete returns `<<xs:schema,xs:sequence` for the completion of sequence
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973335841
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
Review Comment:
I don't think this comment was adressed? Everything else looks good to me.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972990458
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns an empty prefix value
+ if (lineText.includes('<schema')) {
+ return ''
+ }
+ // returns the prefix attached to the schema element plus a colon
+ if (lineText.match(new RegExp('<.*:schema'))) {
+ return lineText.substring(
+ lineText.indexOf('<') + 1,
+ lineText.indexOf(':') + 1
+ )
+ }
Review Comment:
I made a change in the latest version of code
--
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] mbeckerle commented on a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972211402
##########
src/language/providers/utils.ts:
##########
@@ -223,15 +230,16 @@ export function checkSimpleTypeOpen(
document: vscode.TextDocument,
position: vscode.Position
) {
+ const nsPrefix = getNsPrefix(document, position)
var lineNum = position.line
while (lineNum !== 0) {
const wholeLine = document
.lineAt(lineNum)
.text.substr(0, document.lineAt(lineNum).range.end.character)
if (
- wholeLine.includes('<xs:simpleType') &&
- !wholeLine.includes('</xs:simpleType') &&
+ wholeLine.includes('<' + nsPrefix + 'simpleType') &&
Review Comment:
Since this is everywhere, why not define an object that can be called like
`wholeLineElt('simpleType')`
which does this
`wholeLineIncludes('<' + nsPrefix + arg)`
So that the namespace prefix stuff is suppressed.
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns an empty prefix value
+ if (lineText.includes('<schema')) {
+ return ''
Review Comment:
This is fine. Basically following the lead of how the schema element is prefixed.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972916029
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
Review Comment:
but `TextLine` has a `.text` property, the [TextLine doc](https://code.visualstudio.com/api/references/vscode-api#TextLine) says:
> text: string
>
> The text of this line without the line separator characters.
So calling `document.lineAt(lineNum).text` should get the entire line as a string.
--
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 merged pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
Shanedell merged PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294
--
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 pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1256391058
FYI, what you merged changed all the spacing of package.json, and one comment was not addressed. Please don't be hasty about merging PRs and make sure to double check the differences when you force push changes.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r974150149
##########
src/language/providers/utils.ts:
##########
@@ -283,16 +272,35 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document.lineAt(lineNum).text
+ // returns either empty prefix value or a prefix plus a colon
+ let text = lineText.match(schemaPrefixRegEx)
+ if (text != null) {
+ return text[1]
+ }
+ ++lineNum
+ }
+ //returns the standard prefix plus a colon in the case of missing schema tag
+ return 'xs:'
+}
+
export function checkBraceOpen(
document: vscode.TextDocument,
position: vscode.Position
) {
var lineNum = position.line
while (lineNum !== 0) {
- const wholeLine = document
- .lineAt(lineNum)
- .text.substring(0, document.lineAt(lineNum).range.end.character)
+ const wholeLine = document.lineAt(lineNum).text
+ //.text.substring(0, document.lineAt(lineNum).range.end.character)
Review Comment:
Remove comment
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973151524
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
That shouldn't match the regex and should return null and so not return. Running a small test, this outputs `null` for me:
```js
const regex = new RegExp('</?(|[^ ]+:)schema');
const str = '<?xml version="1.0" encoding="UTF-8"?>';
console.log(str.match(regex))
```
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973146919
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
+ let text = lineText.match(schemaPrefixRegEx)
+ if (text != null) {
+ console.log(text[1])
Review Comment:
Removed
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972993031
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
Review Comment:
added additional comments
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973319061
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
It is working for me now.
--
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] scholarsmate commented on pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
scholarsmate commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1252625981
In the commit message we have this:
> add function to parse prefix from schema tag and return the prefix **plas** a colon or return nothing
plus is misspelled as **plas**.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973127992
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
I can test the code without the if statement, but I believe without this test, the code would pass back empty for the first line in the file, which in the examples I have seen is the <?xml version="1.0" encoding="UTF-8"?>.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973127992
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
I can test the code without the if statement, but I believe without this test, the code would pass back empty for the first line in the file, which in the examples I have seen is the `<?xml` version="1.0" encoding="UTF-8"?>.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973147576
##########
src/language/providers/utils.ts:
##########
@@ -17,6 +17,8 @@
import * as vscode from 'vscode'
+const schemaPrefixRegEx = new RegExp('<(.*?)schema')
Review Comment:
Testing shows it works
--
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 pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1252537488
The commit message isn't quite right. These lines should be removed:
```
-renamed function getNsPrefix to getXsdNsPrefix
-added additional comments to getXsdNsPrefix to notify that a colon is include with the returned prefix
-replaced if statement to test for "schema" with a regex
```
The commit does not actually do these things. Those were changes made due to review comments, but in the context of this single squashed commit do not happen.
There's also duplicate sentences in the commit message.
And you only need to include the related issue at the end, with "Closed #122". It does not need to be on the first line.
Also, I'm not sure if the meaning of "Resolves Remove Prefixes" is very clear. Should that be something like "Suggest namespace prefixes based on the schema element prefix"
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r973145818
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +293,31 @@ export function checkSetVariableOpen(
return false
}
+//returns an empty value or a prefix plus a colon
+export function getXsdNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns either empty prefix value or a prefix plus a colon
+ if (lineText.includes('schema')) {
Review Comment:
With the if statement removed autocomplete returns `<<xs:schema,xs:sequence`
--
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 pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
rt320 commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1249719934
Sorry, I missed your comment about forced commits.
--
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 a diff in pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on code in PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#discussion_r972275682
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
+ // returns an empty prefix value
+ if (lineText.includes('<schema')) {
+ return ''
+ }
+ // returns the prefix attached to the schema element plus a colon
+ if (lineText.match(new RegExp('<.*:schema'))) {
+ return lineText.substring(
+ lineText.indexOf('<') + 1,
+ lineText.indexOf(':') + 1
+ )
+ }
Review Comment:
In the other review I suggested a regex that matches both `<foo:schema` and `<schema` so you don't need to do two scans, as well as using a capture group so you don't need to use substring. Did that not work?
Also, maybe move this RegExp to some global state so it doesn't need to be recompiled for iteration everytime this function is called? Feels like taht could be cause a performance hit?
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
+ document: vscode.TextDocument,
+ position: vscode.Position
+) {
+ var initialLineNum = position.line
+ var lineNum = 0
+ while (initialLineNum !== 0 && lineNum <= initialLineNum) {
+ const lineText = document
+ .lineAt(lineNum)
+ .text.substr(0, document.lineAt(lineNum).range.end.character)
Review Comment:
Doesn't this substring just getting the whole line? Why not just use this to get the whole line:
```js
const lineText = document.lineAt(lineNum).text`
```
##########
src/language/providers/utils.ts:
##########
@@ -283,6 +291,33 @@ export function checkSetVariableOpen(
return false
}
+export function getNsPrefix(
Review Comment:
Please add a comment that this returns the prefix + a colon if the prefix is not the empty string. Based on the name I wouldn't normally expect this function to return the colon since namespace prefix is usually just the stuff before the colon. But I understand that including the colon makes the callers of this function much cleaner, so I think it just needs to be documented.
##########
src/language/providers/utils.ts:
##########
@@ -223,15 +230,16 @@ export function checkSimpleTypeOpen(
document: vscode.TextDocument,
position: vscode.Position
) {
+ const nsPrefix = getNsPrefix(document, position)
var lineNum = position.line
while (lineNum !== 0) {
const wholeLine = document
.lineAt(lineNum)
.text.substr(0, document.lineAt(lineNum).range.end.character)
if (
- wholeLine.includes('<xs:simpleType') &&
- !wholeLine.includes('</xs:simpleType') &&
+ wholeLine.includes('<' + nsPrefix + 'simpleType') &&
Review Comment:
Might be worth doing that as part of the fix for #165. I suspect that's going to require a decent refactor anyways.
--
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 pull request #294: Resolves Remove Prefixes Issue #120
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on PR #294:
URL: https://github.com/apache/daffodil-vscode/pull/294#issuecomment-1255395010
There are conflicts that are preventing this from being merged. Can you rebase onto the latest master and fix those conflicts?
Also, while your at it, there's a type in the subject of the commit, there should be a space in `Suggestnamespace`
--
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