-
Notifications
You must be signed in to change notification settings - Fork 44
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
feat(stdlib): Add lookupCI for assocarray and Node #639
Changes from 7 commits
e8ea91d
e148628
44c6f53
29809ef
393c24e
f10202c
d8f2e58
f353bdf
29ea65a
f7d7bba
d8aade1
53bed83
f0a56a7
020bc4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
|
||
constructor(elements: AAMember[]) { | ||
super("roAssociativeArray"); | ||
elements.forEach((member) => | ||
this.elements.set(member.name.value.toLowerCase(), member.value) | ||
); | ||
elements.forEach((member) => this.set(member.name, member.value, true)); | ||
|
||
this.registerMethods({ | ||
ifAssociativeArray: [ | ||
|
@@ -36,6 +34,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
this.keys, | ||
this.items, | ||
this.lookup, | ||
this.lookupCI, | ||
this.setmodecasesensitive, | ||
], | ||
ifEnum: [this.isEmpty], | ||
|
@@ -77,7 +76,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
.map((value: BrsType) => value); | ||
} | ||
|
||
get(index: BrsType) { | ||
get(index: BrsType, isCaseSensitiveGet = false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same suggestion for the name of the variables here and in line 102 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
if (index.kind !== ValueKind.String) { | ||
throw new Error("Associative array indexes must be strings"); | ||
} | ||
|
@@ -93,18 +92,49 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
// method with the desired name separately? That last bit would work but it's pretty gross. | ||
// That'd allow roArrays to have methods with the methods not accessible via `arr["count"]`. | ||
// Same with RoAssociativeArrays I guess. | ||
let indexValue = this.modeCaseSensitive ? index.value : index.value.toLowerCase(); | ||
return this.elements.get(indexValue) || this.getMethod(index.value) || BrsInvalid.Instance; | ||
return ( | ||
this.findElement(index.value, isCaseSensitiveGet) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can get rid of
because if the key is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
this.getMethod(index.value) || | ||
BrsInvalid.Instance | ||
); | ||
} | ||
|
||
set(index: BrsType, value: BrsType) { | ||
set(index: BrsType, value: BrsType, isCaseSensitiveSet = false) { | ||
if (index.kind !== ValueKind.String) { | ||
throw new Error("Associative array indexes must be strings"); | ||
} | ||
let indexValue = this.modeCaseSensitive ? index.value : index.value.toLowerCase(); | ||
// override old key with new one | ||
let oldKey = this.findElementKey(index.value); | ||
if (!this.modeCaseSensitive && oldKey) { | ||
this.elements.delete(oldKey); | ||
} | ||
|
||
let indexValue = isCaseSensitiveSet ? index.value : index.value.toLowerCase(); | ||
this.elements.set(indexValue, value); | ||
return BrsInvalid.Instance; | ||
} | ||
/** if AA is in insensitive mode, it means that we should do insensitive search of real key */ | ||
alimnios72 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private findElementKey(elementKey: string, isCaseSensitiveFind = false) { | ||
let useCaseSensitiveSearch = this.modeCaseSensitive && isCaseSensitiveFind; | ||
elementKey = useCaseSensitiveSearch ? elementKey : elementKey.toLowerCase(); | ||
if (this.elements.has(elementKey)) { | ||
return elementKey; | ||
} | ||
|
||
let realElementKey = undefined; | ||
for (let key of this.elements.keys()) { | ||
if ((useCaseSensitiveSearch ? key : key.toLowerCase()) === elementKey) { | ||
realElementKey = key; | ||
break; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems less than ideal, as it'll cause an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, I even forget about it, a great catch. but there is still be |
||
return realElementKey; | ||
} | ||
|
||
private findElement(elementKey: string, isCaseSensitiveFind = false) { | ||
let realKey = this.findElementKey(elementKey, isCaseSensitiveFind); | ||
return realKey !== undefined ? this.elements.get(realKey) : undefined; | ||
} | ||
|
||
/** Removes all elements from the associative array */ | ||
private clear = new Callable("clear", { | ||
|
@@ -125,7 +155,8 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
returns: ValueKind.Boolean, | ||
}, | ||
impl: (interpreter: Interpreter, str: BrsString) => { | ||
let deleted = this.elements.delete(str.value.toLowerCase()); | ||
let key = this.findElementKey(str.value, this.modeCaseSensitive); | ||
let deleted = key ? this.elements.delete(key) : false; | ||
return BrsBoolean.from(deleted); | ||
}, | ||
}); | ||
|
@@ -142,7 +173,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
returns: ValueKind.Void, | ||
}, | ||
impl: (interpreter: Interpreter, key: BrsString, value: BrsType) => { | ||
this.set(key, value); | ||
this.set(key, value, true); | ||
lkipke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return BrsInvalid.Instance; | ||
}, | ||
}); | ||
|
@@ -166,7 +197,8 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
}, | ||
impl: (interpreter: Interpreter, str: BrsString) => { | ||
let strValue = this.modeCaseSensitive ? str.value : str.value.toLowerCase(); | ||
return this.elements.has(strValue) ? BrsBoolean.True : BrsBoolean.False; | ||
let key = this.findElementKey(str.value, this.modeCaseSensitive); | ||
return key && this.elements.has(key) ? BrsBoolean.True : BrsBoolean.False; | ||
}, | ||
}); | ||
|
||
|
@@ -182,7 +214,9 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
return BrsInvalid.Instance; | ||
} | ||
|
||
this.elements = new Map<string, BrsType>([...this.elements, ...obj.elements]); | ||
obj.elements.forEach((value, key) => { | ||
this.set(new BrsString(key), value, true); | ||
}); | ||
lkipke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return BrsInvalid.Instance; | ||
}, | ||
|
@@ -223,8 +257,22 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
}, | ||
}); | ||
|
||
/** Given a key, returns the value associated with that key. This method is case insensitive. */ | ||
/** Given a key, returns the value associated with that key. | ||
* This method is case insensitive either-or case sensitive, depends on whether `setModeCasesensitive` was called or not. | ||
*/ | ||
private lookup = new Callable("lookup", { | ||
signature: { | ||
args: [new StdlibArgument("key", ValueKind.String)], | ||
returns: ValueKind.Dynamic, | ||
}, | ||
impl: (interpreter: Interpreter, key: BrsString) => { | ||
let lKey = key.value; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I've missed something — why's this variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idk why, but it was as is before my changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
return this.get(new BrsString(lKey), true); | ||
}, | ||
}); | ||
|
||
/** Given a key, returns the value associated with that key. This method always is case insensitive. */ | ||
private lookupCI = new Callable("lookupCI", { | ||
alimnios72 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
signature: { | ||
args: [new StdlibArgument("key", ValueKind.String)], | ||
returns: ValueKind.Dynamic, | ||
|
@@ -242,7 +290,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte | |
returns: ValueKind.Void, | ||
}, | ||
impl: (interpreter: Interpreter) => { | ||
this.modeCaseSensitive = !this.modeCaseSensitive; | ||
this.modeCaseSensitive = true; | ||
return BrsInvalid.Instance; | ||
}, | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove
Get
andSet
from the end of this variables as this can be inferred from the name of the functionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done