Skip to content
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

js.eval, js.evalNS, and js.evalURI use case sensitive comparisons for determining which to run #3011

Closed
cwisniew opened this issue Oct 6, 2021 · 3 comments
Labels
bug tested This issue has been QA tested by someone other than the developer.

Comments

@cwisniew
Copy link
Member

cwisniew commented Oct 6, 2021

Describe the bug
The code for js.eval, js.evalNS, and js.evalURI all check functionName using a case sensitive comparison. As the parser does not check for case when calling the functions this leads to confusing issues if the user attempts to run one of these functions using the wrong case (e.g. js.evalns()).

To Reproduce
Try to execute js.evalURI as js.evaluri and you will get an error as it attempts to parse the URI as javascript.

Expected behaviour
To be consistent with the parser and other MTScript functions these should accept any case.

MapTool Info

  • Latest develop branch
@kwvanderlinde
Copy link
Collaborator

It's worth noting that this isn't specific to the JS functions. E.g., getvbl() will similarly generate an "Unknown function" error. In fact, it seems most of our functions use case-sensitive comparisions based on this quick estimate:

> grep -rn . -e 'functionName' | grep '.equals(' --color=always | wc -l
289
> grep -rn . -e 'functionName' | grep '.equalsIgnoreCase(' --color=always | wc -l
153

@cwisniew
Copy link
Member Author

cwisniew commented Oct 6, 2021

It's worth noting that this isn't specific to the JS functions. E.g., getvbl() will similarly generate an "Unknown function" error. In fact, it seems most of our functions use case-sensitive comparisions based on this quick estimate:

> grep -rn . -e 'functionName' | grep '.equals(' --color=always | wc -l
289
> grep -rn . -e 'functionName' | grep '.equalsIgnoreCase(' --color=always | wc -l
153

oh yay more stuff to fix! :)
I will create a new issue for this. I fixed this one as I was already editing the code (that and I spent 40 minutes wondering why my changes to "Allow URI Access" didn't work when testing). Problem here was no tthe "Unknown function" but it executing the wrong thing...

@Phergus Phergus added this to To do in MapTool 1.10.0 via automation Oct 6, 2021
@Phergus Phergus moved this from To do to In progress in MapTool 1.10.0 Oct 6, 2021
@Phergus Phergus moved this from In progress to Review in progress in MapTool 1.10.0 Oct 6, 2021
@Phergus Phergus added the tested This issue has been QA tested by someone other than the developer. label Oct 7, 2021
@Phergus
Copy link
Contributor

Phergus commented Oct 7, 2021

Tested.

@Phergus Phergus moved this from Review in progress to Reviewer approved in MapTool 1.10.0 Oct 7, 2021
@Phergus Phergus closed this as completed Oct 7, 2021
@Phergus Phergus moved this from Reviewer approved to Done in MapTool 1.10.0 Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tested This issue has been QA tested by someone other than the developer.
Projects
No open projects
Development

No branches or pull requests

3 participants