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

* added worksheet title when processing multi tab sheet #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

* added worksheet title when processing multi tab sheet #12

wants to merge 1 commit into from

Conversation

Ralqawareet
Copy link

hi ,
i was working on reading and converting google sheets data into json and came across your repo , it was really helpful thank you for your time and effort .
one thing i noticed when asking for multi tab worksheet is that i get one big array of all "tab sheets" combined together and in case if you have a lot of tabs inside your sheet things might get messy , so i thought it would be nice to separate them all out in the final json where the output will something like this
{ tabTitle1 : [jsonCells] ,
tabTitle1 : [jsonCells]
}

@bassarisse bassarisse closed this May 9, 2017
@bassarisse bassarisse reopened this May 9, 2017
@bassarisse
Copy link
Owner

Hi there, thanks for the PR!

As you can see from the failing tests, this would break our API, so we would have to release a new major version, but maybe we can resolve this by adding a new option. However, when this option is used, I think the object returned should be something like this:

{
  "title": "Worksheet 1",
  "index": 0,
  "data": (...)
}

So we'll receive an array of objects like this one.

What do you think?

@Ralqawareet
Copy link
Author

yeah sorry i should have paid more attention to the changes i've made and did the test's , but i hadn't enough time and had a bit of difficulties into running the test's !
but yeah that would be really nice and much appreciated if there will an option to pass where the output will in similar what you've mentioned .

@bassarisse
Copy link
Owner

No worries! I'll address this when I have some time. In the meantime, feel free to update this PR or to create a new one.

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

Successfully merging this pull request may close these issues.

2 participants