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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
