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

Checking for invalid chars in define_name() seems to be wrong #82

Closed
martinfranz opened this issue Nov 15, 2013 · 4 comments
Closed

Checking for invalid chars in define_name() seems to be wrong #82

martinfranz opened this issue Nov 15, 2013 · 4 comments
Assignees
Labels

Comments

@martinfranz
Copy link

martinfranz commented Nov 15, 2013

Hi,
the code checks for invalid chars like this (version 0.73 in function define_name):

# Warn if the sheet name contains invalid chars as defined by Excel help.
if ( $name !~ m/^[a-zA-Z_\\][a-zA-Z_.]+/ ) {
    carp "Invalid characters in name '$name' used in defined_name()\n";
    return -1;
}

Excel allows for numbers in the name starting with the second character, so 0-9 should be added to the pattern match.
The check should go until the end of the name, so a $ should be added at the end.
In my opinion the line with the "if" should therefor look like:

if ( $name !~ m/^[a-zA-Z_\][a-zA-Z_0-9.]+$/ ) {

Best regards, Martin
P.S: thanks for that awesome module John!

@ghost ghost assigned jmcnamara Nov 15, 2013
@jmcnamara
Copy link
Owner

Very true, the current rule in Excel::Writer::XLSX is too restrictive. It should probably allow Unicode characters as well.

I'll fix it for the next release.

Thanks,

John

jmcnamara added a commit that referenced this issue Nov 16, 2013
@jmcnamara
Copy link
Owner

Hi Martin,

I've pushed a fix for this to the master branch. If you get a chance you can try it out.

In the end I settled for the following to allow for Unicode matching:

    # Warn if the name contains invalid chars as defined by Excel help.
    if ( $name !~ m/^[\w\\][\w.]*$/ || $name =~ m/^\d/ ) {
        carp "Invalid characters in name '$name' used in defined_name()\n";
        return -1;
    }

I also added some additional checks and a test case for valid/invalid defined names.

If you see any cases that I've missed let me know.

John

jmcnamara added a commit that referenced this issue Nov 16, 2013
jmcnamara added a commit that referenced this issue Nov 16, 2013
@martinfranz
Copy link
Author

Hi John,

I tried your fix with my code and it worked fine.

Thanks for this extremely fast fix!

@jmcnamara
Copy link
Owner

Hi Martin,

Thanks for the feedback. Fixed in version 0.74 which is on its way to CPAN.

Thanks,

John

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants