Add post processing to files generated by Haskell generators #968

wing328 · 2018-09-05T04:06:08Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

If the environment variable HFMT_PATH (e.g. $HOME/.local/bin/hfmt) is defined, use it to format the Haskell code.

~~TODO: run hfmt to format Haskell servant code~~

jonschoning · 2018-09-05T16:27:03Z

btw, are you generating the sample with bin/haskell-http-client-petstore.sh or bin/openapi3/haskell-http-client-petstore.sh ?

jonschoning · 2018-09-05T16:29:04Z

not sure if i like having hfmt (and requiring/directing the user to install the hfmt tool) alter the output of the templates, but i'll look it over and see if it's more or less readable than the current output, a least for the haskell-http-client generator

wing328 · 2018-09-05T16:33:08Z

@jonschoning thanks for the feedback. The formatting done by hfmt can be switched off by removing the environment variable "HFMT_PATH".

I agree with you that some users may not like the output of hfmt so the formatting should remain optional.

wing328 · 2018-09-13T07:34:08Z

UPDATE: added hfmt to format Haskell Servant code, minor bug fix ("object" comparison) and enhancement (add "binary" type mapping) via 5d1dbde

jonschoning · 2018-09-13T18:19:35Z

After looking at some of the differences, I'm not really liking this change for two reasons:

running modifications (i.e. hfmt) on the output of the templates introduces uncertainty into the final output
hfmt sometimes makes very long lines where before there was nice separation
the code that is generated is already very readable, and i'm not sure the changes result in better readability, in some cases it reduces readability.
i would rather make modifications to the templates to fix any readability issues, and so that the output of the templates is deterministic.

if the current output of the source templates was really bad, this would be an improvement, but since the output of the templates have already been written carefully to be pretty readable, i think this just makes things harder to read than easier.

jonschoning · 2018-09-13T18:21:50Z

samples/server/petstore/haskell-servant/lib/OpenAPIPetstore/API.hs

-     (coerce -> loginUser) :<|>
-     (coerce -> logoutUser) :<|>
-     (coerce -> updateUser)) = client (Proxy :: Proxy OpenAPIPetstoreAPI)
+    ((coerce -> addPet) :<|> (coerce -> deletePet) :<|> (coerce -> findPetsByStatus) :<|> (coerce -> findPetsByTags) :<|> (coerce -> getPetById) :<|> (coerce -> updatePet) :<|> (coerce -> updatePetWithForm) :<|> (coerce -> uploadFile) :<|> (coerce -> deleteOrder) :<|> (coerce -> getInventory) :<|> (coerce -> getOrderById) :<|> (coerce -> placeOrder) :<|> (coerce -> createUser) :<|> (coerce -> createUsersWithArrayInput) :<|> (coerce -> createUsersWithListInput) :<|> (coerce -> deleteUser) :<|> (coerce -> getUserByName) :<|> (coerce -> loginUser) :<|> (coerce -> logoutUser) :<|> (coerce -> updateUser)) =


this is basically unreadable

wing328 · 2018-09-14T03:58:19Z

@jonschoning thanks for reviewing the output. What about using stylish-haskell instead, which allows users to provide a config [1] and seems to be more popular [2]?

Ref:
[1] https://github.com/jaspervdj/stylish-haskell#configuration
[2] https://www.reddit.com/r/haskell/comments/7bhsu0/what_code_formatter_are_you_using/

UPDAET: I've updated the samples using stylish-haskell via 1f19a5f

wing328 · 2018-09-20T07:29:16Z

UPDATE: I've renamed the environment variable to "HASKELL_POST_PROCESS_FILE" to make it consistent across all generators. The user can define it with code formatter of their choice or using other tools to post process the auto-generated Haskell files.

jonschoning · 2018-09-20T15:06:26Z

stylish-haskell has better formatting, but it appears to not handle all cases and generates errors:

[main] ERROR o.o.c.l.HaskellHttpClientCodegen - Error running the command (stylish-haskell -i /home/jon/fs/git/openapi-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/Client.hs). Exit value: 1

I like the approach of letting the user choose the code formatter as they wish.
However, at least for haskell-http-client, I already put effort into making the templates generate well-formatted code. How about we let the users choose to use HASKELL_POST_PROCESS_FILE if they wish, but we let the /samples remain the unadjusted output from the templates?

wing328 · 2018-09-20T16:36:22Z

[main] ERROR o.o.c.l.HaskellHttpClientCodegen - Error running the command (stylish-haskell -i /home/jon/fs/git/openapi-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/Client.hs). Exit value: 1

I couldn't repeat the issue locally. Here are partial results:

[main] INFO  o.o.c.l.HaskellHttpClientCodegen - Successfully executed: /Users/williamcheng/.local/bin/stylish-haskell -i /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore.hs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/Client.hs
[main] INFO  o.o.c.l.HaskellHttpClientCodegen - Successfully executed: /Users/williamcheng/.local/bin/stylish-haskell -i /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/Client.hs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/API.hs
[main] INFO  o.o.c.l.HaskellHttpClientCodegen - Successfully executed: /Users/williamcheng/.local/bin/stylish-haskell -i /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/API.hs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /Users/williamcheng/Code/graphql-generator/samples/client/petstore/haskell-http-client/lib/OpenAPIPetstore/Core.hs

Here is how I set the env variable:

export HASKELL_POST_PROCESS_FILE="/Users/williamcheng/.local/bin/stylish-haskell -i"

How about we let the users choose to use HASKELL_POST_PROCESS_FILE if they wish, but we let the /samples remain the unadjusted output from the templates?

Definitely. I updated the samples to show you the difference made by the code formatter. The samples should be not be formatted automatically. I've updated this PR to restore the Haskell samples without using a code formatter.

jonschoning · 2018-09-20T16:47:19Z

I like that.

I saw you used ./bin/haskell-http-client-petstore.sh to generate the samples, where previously the samples were generated with ./bin/openapi3/haskell-http-client-petstore.sh.

which one would you prefer we generate the samples with? oas2 or oas3?

wing328 · 2018-09-20T16:52:28Z

I still use oas2 version to update samples.

We may need to move some samples to another repository instead as the size of the repo will become too big if we include samples generated with OAS3 spec

If this change looks good to you, please feel free to approve and merge it.

jonschoning · 2018-09-20T16:59:07Z

ok, i approve this then.. I will have to make some updates to the example app in the samples because it is compiling against the oas3 version.

…Tools#968) * add hfmt support (without updating the sample) * update haskell httpclient samples with hfmt * add code format option to haskell servant, minor bug fixes * update code samples with hfmt * update samples using stylish-haskell * rename env variable * update haskell samples with stylish-haskell * regenerate haskell samples without stylish-haskell * regenerate haskell servant sample * update example-app & tests-integration for OAS2 code

wing328 added 2 commits September 5, 2018 11:43

add hfmt support (without updating the sample)

9d85827

update haskell httpclient samples with hfmt

dfdde0b

wing328 added Client: Haskell Enhancement: Code format labels Sep 5, 2018

wing328 added this to the 3.3.0 milestone Sep 5, 2018

wing328 added the WIP Work in Progress label Sep 5, 2018

wing328 added 2 commits September 13, 2018 15:29

add code format option to haskell servant, minor bug fixes

5d1dbde

update code samples with hfmt

01035b3

wing328 removed the WIP Work in Progress label Sep 13, 2018

jonschoning reviewed Sep 13, 2018

View reviewed changes

wing328 added 2 commits September 14, 2018 22:33

update samples using stylish-haskell

1f19a5f

rename env variable

3a1c557

wing328 added 4 commits September 21, 2018 00:01

Merge remote-tracking branch 'origin/master' into haskell_code_format

7b2c9d7

update haskell samples with stylish-haskell

61df342

regenerate haskell samples without stylish-haskell

8656e56

regenerate haskell servant sample

e77d68c

update example-app & tests-integration for OAS2 code

ab93f0e

jonschoning force-pushed the haskell_code_format branch from 641021c to ab93f0e Compare September 21, 2018 03:26

wing328 changed the title ~~Use hfmt to format Haskell code~~ Add post processing to files generated by Haskell generators Sep 21, 2018

wing328 merged commit cbddb08 into master Sep 29, 2018

wing328 deleted the haskell_code_format branch September 29, 2018 09:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add post processing to files generated by Haskell generators #968

Add post processing to files generated by Haskell generators #968

wing328 commented Sep 5, 2018 •

edited

Loading

jonschoning commented Sep 5, 2018

jonschoning commented Sep 5, 2018 •

edited

Loading

wing328 commented Sep 5, 2018

wing328 commented Sep 13, 2018 •

edited

Loading

jonschoning commented Sep 13, 2018 •

edited

Loading

jonschoning Sep 13, 2018

wing328 commented Sep 14, 2018 •

edited

Loading

wing328 commented Sep 20, 2018

jonschoning commented Sep 20, 2018 •

edited

Loading

wing328 commented Sep 20, 2018

jonschoning commented Sep 20, 2018

wing328 commented Sep 20, 2018 •

edited

Loading

jonschoning commented Sep 20, 2018

Add post processing to files generated by Haskell generators #968

Add post processing to files generated by Haskell generators #968

Conversation

wing328 commented Sep 5, 2018 • edited Loading

PR checklist

Description of the PR

jonschoning commented Sep 5, 2018

jonschoning commented Sep 5, 2018 • edited Loading

wing328 commented Sep 5, 2018

wing328 commented Sep 13, 2018 • edited Loading

jonschoning commented Sep 13, 2018 • edited Loading

jonschoning Sep 13, 2018

Choose a reason for hiding this comment

wing328 commented Sep 14, 2018 • edited Loading

wing328 commented Sep 20, 2018

jonschoning commented Sep 20, 2018 • edited Loading

wing328 commented Sep 20, 2018

jonschoning commented Sep 20, 2018

wing328 commented Sep 20, 2018 • edited Loading

jonschoning commented Sep 20, 2018

wing328 commented Sep 5, 2018 •

edited

Loading

jonschoning commented Sep 5, 2018 •

edited

Loading

wing328 commented Sep 13, 2018 •

edited

Loading

jonschoning commented Sep 13, 2018 •

edited

Loading

wing328 commented Sep 14, 2018 •

edited

Loading

jonschoning commented Sep 20, 2018 •

edited

Loading

wing328 commented Sep 20, 2018 •

edited

Loading