Aditional configuration request #7

Closed
opened 2016-12-06 02:54:00 +01:00 by alx741 · 10 comments
alx741 commented 2016-12-06 02:54:00 +01:00 (Migrated from github.com)

I'm trying out brittany and really like it, I think it has the potential to beat hindent through configuration capabilities; The latest hindent version completely removes the styles mechanism and is fixed to the "Johan Tibell" one with no more configuration other than indentation size and line length, Johan Tibell's style is pretty good but I'm not always necessarily happy with every way it changes the code.

As I try brittany I found out a couple things that bothers me and that I would like to be able to configure on ~/.brittany/config.yaml. Not to say that the default behavior is bad though:

Guards

Have/Want:

isValidPosition :: Position -> Maybe Position
isValidPosition position
    | validX && validY = Just position
    | otherwise = Nothing

Get:

isValidPosition :: Position -> Maybe Position
isValidPosition position
    | validX && validY
    = Just position
    | otherwise
    = Nothing

Case

Have/Want:

dropTile :: Board -> Position -> Board
dropTile board position =
    case cell of
        Just (Tile _) -> board'
        _ -> board

Get:

dropTile :: Board -> Position -> Board
dropTile board position = case cell of
    Just (Tile _) -> board'
    _ -> board
I'm trying out *brittany* and really like it, I think it has the potential to beat *hindent* through configuration capabilities; The latest *hindent* version completely removes the styles mechanism and is fixed to the "Johan Tibell" one with no more configuration other than indentation size and line length, Johan Tibell's style is pretty good but I'm not always necessarily happy with every way it changes the code. As I try *brittany* I found out a couple things that bothers me and that I would like to be able to configure on `~/.brittany/config.yaml`. Not to say that the default behavior is bad though: ### Guards Have/Want: ```Haskell isValidPosition :: Position -> Maybe Position isValidPosition position | validX && validY = Just position | otherwise = Nothing ``` Get: ```Haskell isValidPosition :: Position -> Maybe Position isValidPosition position | validX && validY = Just position | otherwise = Nothing ``` ### Case Have/Want: ```Haskell dropTile :: Board -> Position -> Board dropTile board position = case cell of Just (Tile _) -> board' _ -> board ``` Get: ```Haskell dropTile :: Board -> Position -> Board dropTile board position = case cell of Just (Tile _) -> board' _ -> board ```
lspitzner commented 2016-12-13 22:06:09 +01:00 (Migrated from github.com)

Sorry for the slow reply, I struggle to find time to work more on brittany. You are totally right about the incomplete Guard layouting, there simply is one case missing from the corresponding translation ruleset. I have run into this exact problem as well, so there is a good chance that I will fix it relatively soon :p

For the case part, I am not sure what exactly you want. The implementation currently roughly works like this: Certain syntactical constructs are translated to a (first line, indented-rest-of-lines-list) thingy.
Examples for applications of this rule include:

foo = case cell of
  ..
foo = do
  ..
foo $ do
  ..
foo $ if expr
  then ..
  else ..
foo $ case abc of
  ..
foo $ bar
  { ..
  }

And while it certainly is possible to disable this, before I implement this I would like to confirm in what instances you actually want this. just for case and after = of a binding?

Sorry for the slow reply, I struggle to find time to work more on brittany. You are totally right about the incomplete Guard layouting, there simply is one case missing from the corresponding translation ruleset. I have run into this exact problem as well, so there is a good chance that I will fix it relatively soon :p For the `case` part, I am not sure what exactly you want. The implementation currently roughly works like this: Certain syntactical constructs are translated to a (first line, indented-rest-of-lines-list) thingy. Examples for applications of this rule include: ~~~~.hs foo = case cell of .. foo = do .. foo $ do .. foo $ if expr then .. else .. foo $ case abc of .. foo $ bar { .. } ~~~~ And while it certainly is possible to disable this, before I implement this I would like to confirm in what instances you actually want this. just for `case` and after `=` of a binding?
lspitzner commented 2016-12-13 22:06:18 +01:00 (Migrated from github.com)

(Note to self: maybe the desired effect is just replacing docForceParSpacing with docForceSingleLine in this line)

(Note to self: maybe the desired effect is just replacing docForceParSpacing with docForceSingleLine [in this line](https://github.com/lspitzner/brittany/blob/508780466c68a94aead96b58ad9f68c4da430cee/src/Language/Haskell/Brittany/Layouters/Decl.hs#L249))
lspitzner commented 2017-03-06 16:12:42 +01:00 (Migrated from github.com)

The "guards" part of this issue should be resolved with that commit.

The "guards" part of this issue should be resolved with that commit.
alx741 commented 2017-03-07 20:36:32 +01:00 (Migrated from github.com)

Great, But I'm not sure what the config.yaml option is to configure that behavior (?)

There is a problem though, it gives an error:

error reading in brittany config from /home/alx/.brittany/config.yaml:
Error in $: key "conf_debug" not present

I've already tried removing the old ~/.brittany/config.yaml and let brittany re-generate it, but the problem persists

Great, But I'm not sure what the `config.yaml` option is to configure that behavior (?) There is a problem though, it gives an error: ``` error reading in brittany config from /home/alx/.brittany/config.yaml: Error in $: key "conf_debug" not present ``` I've already tried removing the old `~/.brittany/config.yaml` and let brittany re-generate it, but the problem persists
lspitzner commented 2017-03-07 20:55:04 +01:00 (Migrated from github.com)

I can reproduce.. that must have been broken since some commits. One sec.

I can reproduce.. that must have been broken since some commits. One sec.
lspitzner commented 2017-03-07 21:16:15 +01:00 (Migrated from github.com)

fix is pushed.

fix is pushed.
lspitzner commented 2017-03-07 21:32:24 +01:00 (Migrated from github.com)

The behaviour for guards is improved "unconditionally", i.e. not behind some config flag. Like in other cases, the layouting changes depending on available space. See the testcases for the new behaviour (these tests confirm that brittany acts as identity on them).

The behaviour for guards is improved "unconditionally", i.e. not behind some config flag. Like in other cases, the layouting changes depending on available space. See [the testcases for the new behaviour](https://github.com/lspitzner/brittany/blob/cea81d53690e6c0afe3bda92a2d1805e9de707be/src-literatetests/tests.blt#L454-L468) (these tests confirm that brittany acts as identity on them).
lspitzner commented 2017-03-07 21:46:30 +01:00 (Migrated from github.com)

(oh, and looking at the testcases, I notice that one is missing. And it really did not work; fix is pushed. But you prob. won't run into that unless you have multiple, long guards.)

(oh, and looking at the testcases, I notice that one is missing. And it really did not work; fix is pushed. But you prob. won't run into that unless you have multiple, long guards.)
alx741 commented 2017-03-07 23:33:36 +01:00 (Migrated from github.com)

Perfect, now it works nicely, thanks!

Perfect, now it works nicely, thanks!
lspitzner commented 2017-05-04 12:48:25 +02:00 (Migrated from github.com)

closing due to no feedback on the "case" part of the initial description; feel free to open again if desired.

closing due to no feedback on the "case" part of the initial description; feel free to open again if desired.
Sign in to join this conversation.
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#7
There is no content yet.