Add instance formatting, defaulting to ExactPrint in places #187

2018-10-10T20:39:24+02:00

ruhatch commented

2018-10-10 20:39:24 +02:00

(Migrated from github.com)

Format typeclass instances, defaulting to ExactPrint for the instance head and type/data family instances. The results from type/data family instances contain newlines/indentation that is stripped using Text processing. This was easier than writing a full formatter for those instances, but should be checked carefully and replaced soon.

Format typeclass instances, defaulting to ExactPrint for the instance head and type/data family instances. The results from type/data family instances contain newlines/indentation that is stripped using `Text` processing. This was easier than writing a full formatter for those instances, but should be checked carefully and replaced soon.

🎉 1

ruhatch commented

2018-10-10 20:39:40 +02:00

(Migrated from github.com)

Relevant to #17

eborden commented

2018-10-10 21:08:53 +02:00

(Migrated from github.com)

Not commenting on code quality, but I've run this on Freckle and commented here:
https://github.com/lspitzner/brittany/issues/17#issuecomment-428694848

Not commenting on code quality, but I've run this on Freckle and commented here: https://github.com/lspitzner/brittany/issues/17#issuecomment-428694848

tfausak commented

2018-10-11 17:54:03 +02:00

(Migrated from github.com)

I ran this against the ITProTV codebase and it looks great! It correctly formatted all our instances, nothing else changed, and everything still built 😄

🎉 1

lspitzner commented

2018-10-11 20:33:29 +02:00

(Migrated from github.com)

Thanks, looks good!

(travis failure is probably a false positive - travis still chokes on the "run tests in parallel" change merged recently. thought i had that resolved, will have to have another look.)

Rg stripWhitespace': Yeah, its a bit hacky, but fine by me. I understand somewhat why exactprint behaves this way, but it is more annoying than useful. Either way, this is a temporary solution anyways (until brittany learns to layout data/type decls). I have pushed a commit that adds a stupidly long explanation, and I have refactored the implementation to be a tad shorter and to avoid any partial functions (Text.init/tail). I am mildly certain that I have not changed semantics, but if you want to make sure you can run tests again (@tfausak).

This is ready to merge, but I will wait for feedback, if you have any.

Thanks, looks good! (travis failure is probably a false positive - travis still chokes on the "run tests in parallel" change merged recently. thought i had that resolved, will have to have another look.) Rg `stripWhitespace'`: Yeah, its a bit hacky, but fine by me. I understand somewhat why exactprint behaves this way, but it is more annoying than useful. Either way, this is a temporary solution anyways (until brittany learns to layout data/type decls). I have pushed a commit that adds a stupidly long explanation, and I have refactored the implementation to be a tad shorter and to avoid any partial functions (Text.init/tail). I am mildly certain that I have not changed semantics, but if you want to make sure you can run tests again (@tfausak). This is ready to merge, but I will wait for feedback, if you have any.

👍 2

ruhatch (Migrated from github.com) reviewed 2018-10-11 21:22:27 +02:00

ruhatch (Migrated from github.com) left a comment

The changes to stripWhitespace' look good to me, especially removing partial functions. Just waiting on a positive test result from @tfausak or @eborden!

The changes to `stripWhitespace'` look good to me, especially removing partial functions. Just waiting on a positive test result from @tfausak or @eborden!

tfausak commented

2018-10-11 22:02:23 +02:00

(Migrated from github.com)

Tests passed! I formatted our entire codebase using Brittany at commit 6dc5561d08 and ran the tests to establish a baseline. Then I re-formatted using commit 38216cdc02. Everything worked!

Tests passed! I formatted our entire codebase using Brittany at commit 6dc5561d0847aac42f26fbed2e3b1a6bba23c75a and ran the tests to establish a baseline. Then I re-formatted using commit 38216cdc02fa13bf2beca957f0ab49b8cb10cb4c. Everything worked!

eborden commented

2018-10-11 23:01:26 +02:00

(Migrated from github.com)

👍 from me.

:+1: from me.

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#187