Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Only keep at most 31 characters for sheetname#831

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
guyonroche merged 1 commit intoexceljs:masterfromkaleo211:master
Jun 3, 2019

Conversation

kaleo211
Copy link
Contributor

AddressingIssue 398.

@alubbe
Copy link
Member

Could you please add 2 tests for this:

  1. a 31 character name is unchanged
  2. a 32 character name is truncated

@alubbe
Copy link
Member

@guyonroche I think we could print a warning when this happens, or maybe even throw an error, asking the user of the library to only submit short names. What do you think?

Copy link
Member

@SiemienikSiemienik left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Could you please add 2 tests for this:

  1. a 31 character name is unchanged
  2. a 32 character name is truncated
  1. two names with more characters than 31, where first 31 characters are equals

eq.

ThisIsAnExampleNameWith32Chars_AThisIsAnExampleNameWith32Chars_B
  1. similar but with first letter different:
A_ThisIsAnExampleNameWith32CharsB_ThisIsAnExampleNameWith32Chars

Because as far as I know, names can't be equals, what was happened in case no 3 after truncating. So it throws an error that names are equals even if they aren't. Please check it.

Also:- Throw error when create worksheet with name exists- Modify some existing tests due to over size worksheet name
@kaleo211
Copy link
ContributorAuthor

A_ThisIsAnExampleNameWith32Chars

Thank you. I broke down those tests, hope this also works.

@guyonroche
Copy link
Collaborator

@alubbe

@guyonroche I think we could print a warning when this happens, or maybe even throw an error, asking the user of the library to only submit short names. What do you think?
I don't think throwing an error is necessary - but something like a console warning would be appropriate.
This looks good - will merge soon

@guyonrocheguyonroche merged commit418ed91 intoexceljs:masterJun 3, 2019
@lirantal
Copy link

@guyonroche just chiming in here after I discovered now that this broke some things for me. In the future, I would consider changes like this one as a breaking change. The problem it introduced is that because it is truncating to 31 characters there is now increased potential for duplicate names which will trigger an error thrown.

Regardless, keep up the great work on this library 👏

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@SiemienikSiemienikSiemienik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@kaleo211@alubbe@guyonroche@lirantal@Siemienik
[8]ページ先頭
©2009-2025 Movatter.jp