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

Use hindent to automate formatting as much as we can #12

Closed
harendra-kumar opened this issue Mar 11, 2018 · 2 comments
Closed

Use hindent to automate formatting as much as we can #12

harendra-kumar opened this issue Mar 11, 2018 · 2 comments
Labels
type:maintenance Has impact on maintainability

Comments

@harendra-kumar
Copy link
Member

@Abhiroop I tried hindent --style johan-tibell --line-length 79 --indent-size 4 --sort-imports on Prelude.hs and the result is not much different from the existing style so I have no problem with whatever changes it suggests except the bug.

Bug

There is an unfixed bug in it that causes haddock to fail. Essentially it changes:

module Streamly.Prelude
    (
    -- * Construction
      nil
    , cons

To

module Streamly.Prelude
    -- * Construction
    ( nil
    , cons

Note the construction heading comment goes out which haddock does not like.

Stylistic differences

Other changes that differ from the current style, I can forget about the minor stuff in favor of full automation of formatting:

  • it removes several blank lines that usually keep to have a better visible separation between comment sections and in the export list. I guess I can live with that.
  • where has a two space indentation, which is fine I like that because where is more visible with this without loss of indentation.
  • It changes single line signatures to multiline like this:
takeWhile :: Streaming t => (a -> Bool) -> t m a -> t m a

becomes

takeWhile
    :: Streaming t
    => (a -> Bool) -> t m a -> t m a

This is less compact but maybe a bit more readable (subjective), so accepted.

  • Some manually aligned code gets unaligned e.g. this code was aligned on equal sign:
        yield a Nothing  = return (Just (a, nil))
        yield a (Just x) = return (Just (a, fromStream x))

It becomes:

        yield a Nothing = return (Just (a, nil))
        yield a (Just x) = return (Just (a, fromStream x))
  • guards that were on a single line go to the next line:
        let yield a Nothing  | p a       = yld a Nothing
                             | otherwise = stp
            yield a (Just x) | p a       = yld a (Just (go x))
                             | otherwise = stp

becomes

            let yield a Nothing
                    | p a = yld a Nothing
                    | otherwise = stp
                yield a (Just x)
                    | p a = yld a (Just (go x))
                    | otherwise = stp

Not a big deal I guess.

  • It does not keep DoAndIfThenElse style if-then-else:
        in if n1 <= 0
           then (runStream m1) ctx stp yld
           else (runStream m1) ctx stp yield

became

            in if n1 <= 0
                   then (runStream m1) ctx stp yld
                   else (runStream m1) ctx stp yield

I like the first style better.

  • I tend to use if-then-else in a single line where it is possible:
            yield a (Just x) = if a == e then return True else go x

became

            yield a (Just x) =
                if a == e
                    then return True
                    else go x

I am ok with if it can at least follow the DoAndIfThenElse style.

  • I use a let/in like this, where the ends of the keywords align not the beginning:
        let yield a Nothing  = return $ done a
         in (runStream m1) Nothing undefined yield

becomes

        let yield a Nothing = return $ done a
        in (runStream m1) Nothing undefined yield

Minor thing.

Conclusion

Other than the bug and the DoAndIfThenElse I am ok with the other changes. There is already an issue raised on hindent for the bug, maybe we can raise an issue for supporting the DoAndIfThenElse style.

@Abhiroop if you want to have a go at it you can submit a PR. If we change the style then it will be nice if we change all files to match the same style, it can be one at a time though.

@Abhiroop
Copy link
Collaborator

@harendra-kumar Yes I faced the module header bug last night when trying to build Abhiroop@351a378

@harendra-kumar harendra-kumar added the type:maintenance Has impact on maintainability label Mar 21, 2018
adithyaov added a commit to adithyaov/streamly that referenced this issue Oct 16, 2019
# This is the 1st commit message:

.gitignore changed

# This is the commit message composewell#2:

Added ScopedTypeVariables modifier

# This is the commit message composewell#3:

Changed .gitignore

# This is the commit message composewell#4:

Removed List, making branch consistent

# This is the commit message composewell#5:

Added length to DList

# This is the commit message composewell#6:

Eliminated Null

# This is the commit message composewell#7:

Modified DList to use C-malloc

# This is the commit message composewell#8:

Fails: Need to make a proper finalizer

# This is the commit message composewell#9:

Came close to creating a finalizer

Encountered:
ghc: error: a C finalizer called back into Haskell.
This was previously allowed, but is disallowed in GHC 6.10.2 and later.
To create finalizers that may call back into Haskell, use
Foreign.Concurrent.newForeignPtr instead of Foreign.newForeignPtr.
cabal: repl failed for streamly-0.6.1.

# This is the commit message composewell#10:

Using Foreign.Concurrent

# This is the commit message composewell#11:

Solved double free / corruption error

# This is the commit message composewell#12:

Removed redundant constraint

# This is the commit message composewell#13:

Appvyor satisfaction; commented REPL testing

# This is the commit message composewell#14:

Made IsList and folds
@harendra-kumar
Copy link
Member Author

Inactive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:maintenance Has impact on maintainability
Projects
None yet
Development

No branches or pull requests

2 participants