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

Parsing error for some cases of named expressions with numbers (before: Named expression on format "HF2_2" not allowed) #1058

Closed
jboysen opened this issue Sep 7, 2022 · 11 comments · Fixed by #1130
Assignees
Labels
Bug Something isn't working Docs Improvements or additions to documentation Impact: High

Comments

@jboysen
Copy link

jboysen commented Sep 7, 2022

But there is not information about why, and the naming doesn't violate the rules mentioned here: https://hyperformula.handsontable.com/guide/named-expressions.html#naming-convention

A jsfiddle to illustrate the issue: https://jsfiddle.net/uk1djxpw/1/

@sequba
Copy link
Contributor

sequba commented Sep 7, 2022

Hi, @jboysen. I am sorry you experienced an issue with HyperFormula. You are right, the naming rules for named expressions must be updated. Thank you for reporting it.

@sequba sequba self-assigned this Sep 7, 2022
@sequba sequba added Docs Improvements or additions to documentation Impact: Low Bug Something isn't working labels Sep 7, 2022
@jboysen
Copy link
Author

jboysen commented Sep 7, 2022

The implementation of isNameValid indicates that the name is valid:

public isNameValid(expressionName: string): boolean {
if (/^[A-Za-z]+[0-9]+$/.test(expressionName)) {
return false
}
return /^[A-Za-z\u00C0-\u02AF_][A-Za-z0-9\u00C0-\u02AF._]*$/.test(expressionName)
}

Is the implementation incorrect?

@sequba
Copy link
Contributor

sequba commented Sep 8, 2022

It might be incorrect or there is something else going on here. Our team will investigate this issue shortly.

@sequba sequba changed the title Named expression on format "HF2_2" not allowed Parsing error when named expression contains numbers (before: Named expression on format "HF2_2" not allowed) Nov 9, 2022
@sequba
Copy link
Contributor

sequba commented Nov 9, 2022

Also, the heading Naming convention in our named expressions guide is very misleading. What this section describes is not a convention. These are the requirements for a valid name of the expression. @kirszenbaum proposed to change the heading to Custom name rules.

@AMBudnik
Copy link
Contributor

AMBudnik commented Nov 9, 2022

@jboysen
Copy link
Author

jboysen commented Nov 9, 2022

I see now the title changed of this issue. We looked more into this, and found that "HF2_2" is problematic, but "HF_2" was not, so the title is now misleading.

For now we solved it by checking our named expressions like this in a validate method:

if (/^[A-Z]+\d+/ig.test(variable)) {
    return false;
}

@sequba sequba changed the title Parsing error when named expression contains numbers (before: Named expression on format "HF2_2" not allowed) Parsing error for some cases of named expressions with numbers (before: Named expression on format "HF2_2" not allowed) Nov 9, 2022
@sequba
Copy link
Contributor

sequba commented Nov 9, 2022

@jboysen Thank you for your input. It will come in handy in the process of working on this issue.

@sequba
Copy link
Contributor

sequba commented Nov 10, 2022

Possible duplicate: #880

@sequba
Copy link
Contributor

sequba commented Mar 3, 2023

@jboysen,

I'm more than happy to announce that we just released HyperFormula 2.3.1 where this issue is fixed. We are closing this issue as solved. If there is anything that won't work for you after updating, please leave a comment.

@sequba sequba closed this as completed Mar 3, 2023
@kevinplummer
Copy link

kevinplummer commented Mar 14, 2023

Because HF can handle more columns than Excel something like AUFRINGES1 named range expression is invalid in HF but not Excel. How about if we set MaxColumns to say 26 or max Excel columns could you only do the named range check up to that column for compatibility?

@AMBudnik
Copy link
Contributor

Hi @kevinplummer that's an interesting idea. But at the moment, it is not available out of the box. If you'd like to test a custom solution and create a pull request we would be more than happy to review it as at the moment we focus more on fixing issues than on providing new functionalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Docs Improvements or additions to documentation Impact: High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants