misterspeedy / FsExcel

An F# Excel spreadsheet generator
MIT License
142 stars 18 forks source link

Feature Request: Update existing workbook #9

Closed johncj-improving closed 2 years ago

johncj-improving commented 2 years ago

I would like to use this library to make changes to an existing workbook. Ideally, I would like to be able to specify an existing workbook, and then identify one or more existing worksheets to update and/or create one or more new worksheets.

Externally, I would expect the API to look like this:

// assume path is a string pointing to a valid existing Excel file
let myWorkbook = new XLWorksheet(path)

[ 
  Workbook myWorkbook //only works as the first item, otherwise it's a no-op
  Worksheet "wsName" //if a sheet named wsName exists, it becomes the current sheet, if not, it is created
  Cell [ String "Hello, World!" ]
]

I'd be happy to submit a PR for this.

misterspeedy commented 2 years ago

Completely brilliant idea, and one I hadn't even begun to think of. I'd very much welcome a PR.

I don't have official contributor guidelines but here are some pointers:

a) Every feature should be demo'd in Tutorial.dib. (This might be interesting if we are assuming an existing workbook for your new feature, but the cell script could create it.)

b) Edit Tutorial.dib, not ReadMe.md There is a dotnet watch which generates the readme from the tutorial. You can tell if this has worked because soon after you save the .dib you should see two files in your git change list. Let me know if this doesn't work or if you are not using VS Code. (In VS Code you may need to opt into run-on-folder-open: https://stackoverflow.com/questions/58866184/what-does-on-folder-open-mean-in-vscodes-run-on-folder-open-tasks).

c) Similarly on every save of the dib, a script is generated and run which creates all the 'actual' workbooks created by running each cell in the tutorial. These are then used by a big regression test which compares these actuals with expected files checked in in Tests/RegressionTests/Expected. So... if you make a change to the tutorial which creates additional workbook files that you want to include in the test, put a copy of the file(s) in Tests/RegressionTests/Expected. You will also need to have the following as the last line of the markdown before the tutorial cell in question:

<!-- Test -->

To run the tests just do a dotnet test from src/.

d) Generally each tutorial cell should be followed by an image of the resulting spreadsheet, suitably cropped. Add the image into assets\ and point to it in the markdown as we already do in other cells. Obviously the image won't actually show until committed and pushed, because the image link is into github.

I hope this makes sense. It is rather elaborate but I do like the idea of the tutorial, readme and tests all being the same thing.

Thank you so much for the idea, looking forward to the PR!

misterspeedy commented 2 years ago

Forgot to tag you @johncj-improving ^

johncj-improving commented 2 years ago

There appears to be a time zone issue with the tests. I'm getting an error on the regression tests: Expected: 2022-03-12T00:00:00.0000000 Actual: 2022-03-11T18:00:00.0000000

And I've checked the actuals and everywhere the tests create a value like this: System.DateTime(2022, 3, 12) The spreadsheet has the value: 3/11/2022 6:00:00 PM

And yes, my time zone offset is UTC -6. Do you want me to open a bug?

misterspeedy commented 2 years ago

Yes please. Well spotted! Thank you!

On Fri, 11 Mar 2022 at 16:36, johncj-improving @.***> wrote:

There appears to be a time zone issue with the tests. I'm getting an error on the regression tests: Expected: 2022-03-12T00:00:00.0000000 Actual: 2022-03-11T18:00:00.0000000

And I've checked the actuals and everywhere the tests create a value like this: System.DateTime(2022, 3, 12) The spreadsheet has the value: 3/11/2022 6:00:00 PM

And yes, my time zone offset is UTC -6. Do you want me to open a bug?

— Reply to this email directly, view it on GitHub https://github.com/misterspeedy/FsExcel/issues/9#issuecomment-1065283029, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGDPLUDFRUDATRXCXT4HYDU7NZCPANCNFSM5QK5AYEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

misterspeedy commented 2 years ago

PR merged, with very many thanks @johncj-improving. Would welcome more ideas!