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.

Sign up for GitHub

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

Jump to bottom

Fix "Invalid character / found in sheet title" during export to XLSX #490

Merged
merged 3 commits into from
Mar 1, 2021
Merged

Fix "Invalid character / found in sheet title" during export to XLSX #490

merged 3 commits into from
Mar 1, 2021

Conversation

mpasternak
Copy link
Contributor

@mpasternak mpasternak commented Feb 28, 2021 •
edited by hugovk
Loading

Fixes #489.

Make sure there are no bad characters in sheet names!

@codecov
Copy link

codecov bot commented Feb 28, 2021 •
edited
Loading

Codecov Report

Merging #490 (635811d) into master (7035d79) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #490      +/-   ##
==========================================
+ Coverage   90.67%   90.69%   +0.02%     
==========================================
  Files          28       28              
  Lines        2616     2623       +7     
==========================================
+ Hits         2372     2379       +7     
  Misses        244      244
Impacted Files Coverage Δ
src/tablib/formats/_xlsx.py 96.93% <100.00%> (+0.13%) ⬆️
tests/test_tablib.py 98.56% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7035d79...635811d. Read the comment docs.

claudep
claudep requested changes Feb 28, 2021
View reviewed changes
Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

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

Could you please include a small description in the commit first line, after the issue number?

src/tablib/formats/_xlsx.py Outdated
@@ -52,6 +67,8 @@ def export_book(cls, databook, freeze_panes=True):
ws = wb.create_sheet()
ws.title = dset.title if dset.title else 'Sheet%s' % (i)

#

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess those two lines should not be in the patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks

@mpasternak
Copy link
Contributor Author

Could you please include a small description in the commit first line, after the issue number?

I am not sure if I understand, what you mean.

Do you want me to change the commit message? Is this even possible? If it is, I have no idea how can I do that.

I could re-write everything (as this is a very small update) and re-commit with a better message, but is that what you want from me?

Please explain. I am willing to work with you on this patch but I am not sure if I understood correctly.

@claudep
Copy link
Contributor

claudep commented Mar 1, 2021

Exactly, I'd like a small description of the change in the commit message.

You can do that by:
git commit --amend (and editing the message)
Then force push your branch:
git push -f <repo-alias> master

@claudep
Copy link
Contributor

claudep commented Mar 1, 2021

Oh sorry, now I realize that you have several commits already. So you should use some rebase magic to do that.

git rebase -i HEAD~3 would allow you to re-edit the commit message. I'd suggest you to study a bit the rebase capabilities, as it could surely be useful in your projects. Of course, rebasing should be done with caution and only on "working" branches.

@hugovk hugovk changed the title Fix #489 Fix "Invalid character / found in sheet title" during export to XLSX Mar 1, 2021
@mpasternak
Copy link
Contributor Author

I did rebase and a force-push. Does it look okay for you?

claudep
claudep approved these changes Mar 1, 2021
View reviewed changes
Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

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

Perfect, thanks a lot!

@claudep claudep merged commit ebd2e08 into jazzband:master Mar 1, 2021
marisn added a commit to marisn/tablib that referenced this pull request Nov 2, 2021
@marisn
Correct order of arguments to a regex call in safe_xlsx_sheet_title +…
5531beb 
… correct tests

Fixes jazzband#489
Related to jazzband#490
@marisn marisn mentioned this pull request Nov 2, 2021
Merged
claudep pushed a commit that referenced this pull request Nov 4, 2021
@marisn
Correct order of arguments to a regex call in safe_xlsx_sheet_title
91beb35 
Fixes #489
Related to #490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claudep claudep claudep 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.

"Invalid character / found in sheet title" during export to XLSX
2 participants
@mpasternak @claudep