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

Call GetSheetName can not get correct sheetname #485

Closed
virteman opened this issue Sep 18, 2019 · 15 comments
Closed

Call GetSheetName can not get correct sheetname #485

virteman opened this issue Sep 18, 2019 · 15 comments

Comments

@virteman
Copy link

virteman commented Sep 18, 2019

Description

GetSheetName can not get correct sheet name just return empty string

Steps to reproduce the issue:

  1. my xlsx file's first sheet named "basic". so I call GetSheetIndex("basic") get return value is 15
  2. call GetSheetName(15) return value is "" (empty string)

Describe the results you received:
I get an empty string as sheetname

Describe the results you expected:
I expect the return string should be "basic"

Output of go version:

go version go1.11 linux/amd64

Excelize version or commit ID:

eef232f09ecd41b1f8fc199906ce0be64865802e

Environment details (OS, Microsoft Excel™ version, physical, etc.):

Ubuntu 18.04, wps 2019( can open and eidt excel 2007 xlsx file), thinkpad x1 carbon
@xuri
Copy link
Member

xuri commented Sep 19, 2019

Hi @virteman, thanks for your feedback. Could you provide testing file attachment?

@mlh758
Copy link
Contributor

mlh758 commented Nov 5, 2019

This is likely related to the changes I made here since the old method of using SheetID was causing the bug in #457

It looks like we need to update GetSheetIndex to actually return the index rather than the SheetID since SheetID isn't a reliable index.

@mariaefi29
Copy link

mariaefi29 commented Apr 21, 2020

Dear developers,

Do you have any updates on the issue?

I have just moved to your package from another xlsx parser and when started testing on real data, it turned out it doesn't work for a lot of files.

For instance, I have a file with the only one sheet. I would like to get that sheet and I use file.GetSheetName(1), assuming that the sheet's name will return to me. Unfortunately, it returns an empty string. I dig deeper and I realised that the only one sheet I have in that file has index 2 (it's SheetID equals 2 Так выглядит xlsxWorkbook : Sheets:{Sheet:[{Name:ThirdPartHotelst1.xlsx SheetID:2 ID:rId1 State:}]}). That's weird. As you said, for some reason it seems that SheetID isn't reliable index, but it has 'rId1' real id.

Could you please clarify that and/or give us some ETA on the fix? I used the latest version v2.1.0.

You can get the file here (I filled it up with the fake data).
file to download

We would be grateful you could look into that.

PS. Do you also know why I don't see the commit mentioned above in a released version?

@xuri
Copy link
Member

xuri commented Apr 21, 2020

Hi @mariaefi29, thanks for your feedback, the ID returned by GetSheetMap doesn't represent the order of the worksheet in the workbook. It just a reliable identifier, which can be used to set the active worksheet. Indeed, we couldn't get the order of worksheet in Excelize 2.1.0 currently, I plan to add a new API named GetSheetList in the next version to solve this issue.

@mariaefi29
Copy link

mariaefi29 commented Apr 21, 2020

@xuri,

Thank you for your reply! Very confusing :) Could you please advice us what to do? How can we get the only one sheet in excel file if we don't know the name of the sheet?f.GetSheetName(f.GetActiveSheetIndex())?

So do I understand right that index is not index in its way, but just an ID of the sheet and it can be sort of random?

@mlh758
Copy link
Contributor

mlh758 commented Apr 21, 2020

Right now it's inconsistent, per my comment above. I had implemented a partial fix in #457 via #463 which was moving everything to be index based but missed a couple parts of the API. It's been a while since I looked at this but I think Xuri decided to roll back my change in #486 and make everything work off the SheetID instead.

@xuri
Copy link
Member

xuri commented Apr 21, 2020

Thank you for your reply! Very confusing :) Could you please advice us what to do? How can we get the only one sheet in excel file if we don't know the name of the sheet?f.GetSheetName(f.GetActiveSheetIndex())?

So do I understand right that index is not index in its way, but just an ID of the sheet and it can be sort of random?

Yes, as you said, ID is just the identifier of the worksheet inside the spreadsheet, it doesn't start with 1.

@mariaefi29
Copy link

Dear developers,

Thank you for your replies!

@xuri Do you have any ETA on GetSheetList? We would appreciate it!

@xuri
Copy link
Member

xuri commented Apr 22, 2020

The GetSheetList will be released in the next version 2.2.0 on May 11th, 2020.

@mariaefi29
Copy link

Thank you for this information!

@mariaefi29
Copy link

Would you like to consider change naming for major release version later? Index implies that there be some kind of an order. Sheet's index seems like really a sheet's number, not an identifier, even though it can be an identifier. But id may not be index.

@xuri
Copy link
Member

xuri commented Apr 22, 2020

Yes, currently, the "Index" of the API is actually an "ID", the next version will use a real index instead of an ID. The following functions will be affected:

GetSheetName
GetSheetIndex
GetActiveSheetIndex
SetActiveSheet

@mariaefi29
Copy link

Cool! Thanks!

@xuri xuri closed this as completed in 1fe660d Apr 22, 2020
@xuri
Copy link
Member

xuri commented Apr 23, 2020

Hi @mariaefi29, I have added a new function GetSheetList and make existing API using the worksheet index in the list order. You can upgrade to the master branch code before the version released.

@mariaefi29
Copy link

mariaefi29 commented Apr 23, 2020

Dear @xuri,

Thank you very much! I have tested it on the files from my flow and everything has worked as expected!

nullfy pushed a commit to nullfy/excelize that referenced this issue Oct 23, 2020
-  added 3 internal function: getSheetID, getActiveSheetID, getSheetNameByID
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

No branches or pull requests

4 participants