microlens as an alternative to lens

qrilka / xlsx

Simple and incomplete Excel file parser/writer

MIT License

128 stars 62 forks source link

microlens as an alternative to lens #133

Closed goodlyrottenapple closed 3 years ago

goodlyrottenapple commented 3 years ago

Hi,

This is a PR addressing #129. The only changes, other than using LANGUAGE CPP to import microlens instead of lens are:

In src/Codec/Xlsx/Lens.hs, I unfolded the defn of sheetList in ixSheet and atSheet, since microlens does not provide Iso.

In src/Codec/Xlsx/Types.hs the following

emptyStyles = Styles "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\
\<styleSheet xmlns=\"http://schemas.openxmlformats.org/spreadsheetml/2006/main\"></styleSheet>"

would not play nice with the CPP pragma, so I moved the string onto a single line.

qrilka commented 3 years ago

thanks @goodlyrottenapple for the PR hopefully I'll have time tomorrow to review it properly

qrilka commented 3 years ago

The PR looks good. Would you mind two minor improvements:

squash commits so history will look a bit cleaner
add 1 more entry into Travis matrix with microlens. Probably 8.6.5 should be not old enough to get used.

After that I'll be happy to release this as 0.8.2

goodlyrottenapple commented 3 years ago

Squashed commits and added a --flag xlsx:microlens build for 8.6.5. I had to tweak the travis file further, because the --flag command only seems to be recognised if it comes after test in stack test... Let me know if there is anything else.

qrilka commented 3 years ago

That's perfect, thank you

qrilka commented 3 years ago

And released as 0.8.2

goodlyrottenapple commented 3 years ago

Thanks!