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

cli: Fix compilation without LANG/LC_ALL set #849

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

zarybnicky
Copy link
Contributor

@zarybnicky zarybnicky commented Jul 28, 2018

Embedded files are now assumed to be UTF-8 independently of the system locale. This also replaces the file-embed dependency with Hledger.Cli.TH, where embedStringFile has been changed to assume UTF-8 (utf8_bom).

Compilation without a locale (e.g. in nix-shell --pure or docker run -it fedora:28 bash as @talex5 mentioned) now works without a problem.

I've also changed the nix: sections of stack.yamls to use pure nix shells, and added the required dependencies of happy/alex - stack build --nix works for all build configs. (I'm on NixOS, so I can't actually develop without --nix, and compiling in a pure shell is also a nice bonus.)

Fixes #420, #813. This is also a better solution to the closed #669.

@simonmichael simonmichael added tools hledger developer tools, scripts, processes.. i18n Internationalisation/localisation-related. labels Aug 2, 2018
@simonmichael
Copy link
Owner

@zarybnicky, thanks for making the build more robust. Could you perhaps:

  • submit embedStringFileUtf8 upstream - it seems like a useful feature.
  • include the notes above in the commit description(s)
  • separate the nix/stack updates into their own commit (or, mention why they belong with this one)

@simonmichael simonmichael added the needs:changes To unblock: needs some changes made, in line with recommendations label Aug 2, 2018
@zarybnicky
Copy link
Contributor Author

zarybnicky commented Aug 8, 2018

I've opened an upstream issue, snoyberg/file-embed#27. I've also added to the commit description, and removed the Nix-related changes (I might open another pull request later, it's nothing urgent, just a nice-to-have).

@simonmichael
Copy link
Owner

Thanks @zarybnicky. I guess we should do a shorter workaround with less code duplication following the suggestions on that issue.

@zarybnicky
Copy link
Contributor Author

Oops, didn't mean to click that.

@zarybnicky zarybnicky reopened this Aug 24, 2018
@zarybnicky
Copy link
Contributor Author

Alright, third time's the charm. This version just replaces embedStringFile with embedFile, and uses Data.ByteString functions to print out the docs.

@simonmichael
Copy link
Owner

Great, I guess this fixes this and this and is ready for merge ?

@zarybnicky
Copy link
Contributor Author

Yup, I think so.

@simonmichael simonmichael merged commit 1190e2a into simonmichael:master Aug 24, 2018
@simonmichael
Copy link
Owner

Thanks!

@simonmichael simonmichael added A-WISH Some kind of improvement request, hare-brained proposal, or plea. A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. and removed needs:changes To unblock: needs some changes made, in line with recommendations A-WISH Some kind of improvement request, hare-brained proposal, or plea. labels Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. i18n Internationalisation/localisation-related. tools hledger developer tools, scripts, processes..
Projects
None yet
2 participants