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

Prototype and test idea #59

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

Prototype and test idea #59

wants to merge 1 commit into from

Conversation

tombooth
Copy link

I've thrown together a little something that tests a couple of cases. Please comment on this, definitely less than ideal!

#58

@tombooth tombooth changed the title Prototype to test idea Prototype and test idea Nov 18, 2016
@mjul
Copy link
Owner

mjul commented Nov 22, 2016

Hi Tom,

thanks for writing this. What are the things you consider "less than ideal"?

Would it look better, if we rethought the API for a more fluent compositional style instead of staying close to the V1 API?

The fonts and styles are not the highest priority, but I think they can be rewritten in a better style with the new API by e.g. assigning a name (e.g. :headline) when creating them. This could be stored in the Clojure state map and the the symbolic name could then be used to access them when they are applied.

@mjul
Copy link
Owner

mjul commented Nov 22, 2016

On a further note, I think we could provide sensible defaults to the functions, for example, the -seqfunctions would not need to require more than a workbook. We could have a cursor concept and set it to default to the first cell of the first row of the first sheet: :cursor {:row 1, :cell :a, :sheet "Sheet1"} and then se that as a reference: row-seq would then enumerate the rows of the current sheet selected by the :cursor :sheet (navigating there in the POI instance as needed) so we would not need all the assert about where we are etc.

@mjul
Copy link
Owner

mjul commented Nov 22, 2016

PS: please submit pull requests for this against the v2 branch, not the master.

@tombooth tombooth changed the base branch from master to v2 November 22, 2016 09:34
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