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

fix: allow lowercase cell reference in GET DIRECT function calls #396

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

chrispcampbell
Copy link
Contributor

Fixes #395

This is a simple fix that allows GET DIRECT ... calls to use a cell reference that has lowercase letters instead of the more typical uppercase ones. Vensim is lenient and allows either form, but the XLSX library we use in SDE is not lenient, so I've added a toUpperCase() call in the appropriate places.

I also added some error handling so that users will get a more useful error message in the unlikely case where an invalid cell reference is used (instead of allowing things to pass through).

I updated the existing integration tests and also added some new unit tests to verify this.

Copy link
Collaborator

@ToddFincannonEI ToddFincannonEI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a straightforward fix — also good to see the extra error checks and new test coverage.

@chrispcampbell chrispcampbell merged commit 76f890c into main Nov 15, 2023
6 checks passed
@chrispcampbell chrispcampbell deleted the chris/395-lowercase-cell-refs branch November 15, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow lowercase cell reference in GET DIRECT ... function calls
2 participants