Whitespace added between comment and multi-line where clause #263

New Issue

2019-11-11T21:14:37+01:00

stackptr commented

2019-11-11 21:14:37 +01:00

(Migrated from github.com)

It appears when formatting a function with a long comment about a multi-line where clause, brittany will add a bunch of whitespace that seems somehow related to the length of the comment.

Input this into the online formatter (some function body has been omitted for brevity):

validateBasic
  :: forall  m . (MonadThrow m, HasCallStack) => BasicQuestion -> m ()
validateBasic BasicQuestion {..} = do
  when (basicQuestionAnswerType == Just SingularOr) $ do
    let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers

  -- There are a ton of other validations that would be nice to have, like:
  -- - Are the correct answers formatted properly? (Should be answers separated
  --   by space-pipe-space, like `x | y | z`.)
  -- - Do any params not get substituted? In other words, are there any non-
  --   `Nothing` params that don't have a corresponding Mustache/ERB
  --   substitution?
  -- - Do any fields other than the question text "look like" question text?
  --   The easiest way to tell would be to look for Mustache/ERB tags.
 where
  isMultiple =
    basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice]

The output will show whitespace between the last line of the comment and the where:

validateBasic
  :: forall  m . (MonadThrow m, HasCallStack) => BasicQuestion -> m ()
validateBasic BasicQuestion {..} = do
  when (basicQuestionAnswerType == Just SingularOr) $ do
    let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers

  -- There are a ton of other validations that would be nice to have, like:
  -- - Are the correct answers formatted properly? (Should be answers separated
  --   by space-pipe-space, like `x | y | z`.)
  -- - Do any params not get substituted? In other words, are there any non-
  --   `Nothing` params that don't have a corresponding Mustache/ERB
  --   substitution?
  -- - Do any fields other than the question text "look like" question text?
  --   The easiest way to tell would be to look for Mustache/ERB tags.









 where
  isMultiple =
    basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice]

Further formatting will result in more whitespace.

The comment above is 8 lines and results in 9 lines of whitespace. Trimming the comment to 4 lines results in 5 lines of whitespace.

Shortening the definition of isMultiple (for instance, removing Just MultipleChoice from the list) will result in it formatting to a single line with no whitespace.

It appears when formatting a function with a long comment about a _multi-line_ `where` clause, brittany will add a bunch of whitespace that seems somehow related to the length of the comment. Input this into the [online formatter](https://hexagoxel.de/brittany/) (some function body has been omitted for brevity): ```haskell validateBasic :: forall m . (MonadThrow m, HasCallStack) => BasicQuestion -> m () validateBasic BasicQuestion {..} = do when (basicQuestionAnswerType == Just SingularOr) $ do let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers -- There are a ton of other validations that would be nice to have, like: -- - Are the correct answers formatted properly? (Should be answers separated -- by space-pipe-space, like `x | y | z`.) -- - Do any params not get substituted? In other words, are there any non- -- `Nothing` params that don't have a corresponding Mustache/ERB -- substitution? -- - Do any fields other than the question text "look like" question text? -- The easiest way to tell would be to look for Mustache/ERB tags. where isMultiple = basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice] ``` The output will show whitespace between the last line of the comment and the `where`: ```haskell validateBasic :: forall m . (MonadThrow m, HasCallStack) => BasicQuestion -> m () validateBasic BasicQuestion {..} = do when (basicQuestionAnswerType == Just SingularOr) $ do let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers -- There are a ton of other validations that would be nice to have, like: -- - Are the correct answers formatted properly? (Should be answers separated -- by space-pipe-space, like `x | y | z`.) -- - Do any params not get substituted? In other words, are there any non- -- `Nothing` params that don't have a corresponding Mustache/ERB -- substitution? -- - Do any fields other than the question text "look like" question text? -- The easiest way to tell would be to look for Mustache/ERB tags. where isMultiple = basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice] ``` Further formatting will result in _more_ whitespace. The comment above is 8 lines and results in 9 lines of whitespace. Trimming the comment to 4 lines results in 5 lines of whitespace. Shortening the definition of `isMultiple` (for instance, removing `Just MultipleChoice` from the list) will result in it formatting to a single line with no whitespace.

👍 1

eborden commented

2019-11-12 01:18:10 +01:00

(Migrated from github.com)

There also seems to be an issue here of indentation level. The code above is using special where indentation (1 space), but the comment is being indented with the do block (2 space). @stackptr what would you expect?

this

validateBasic
  :: forall  m . (MonadThrow m, HasCallStack) => BasicQuestion -> m ()
validateBasic BasicQuestion {..} = do
  when (basicQuestionAnswerType == Just SingularOr) $ do
    let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers

  -- There are a ton of other validations that would be nice to have, like:
  -- - Are the correct answers formatted properly? (Should be answers separated
  --   by space-pipe-space, like `x | y | z`.)
  -- - Do any params not get substituted? In other words, are there any non-
  --   `Nothing` params that don't have a corresponding Mustache/ERB
  --   substitution?
  -- - Do any fields other than the question text "look like" question text?
  --   The easiest way to tell would be to look for Mustache/ERB tags.
 where
  isMultiple =
    basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice]

or

validateBasic
  :: forall  m . (MonadThrow m, HasCallStack) => BasicQuestion -> m ()
validateBasic BasicQuestion {..} = do
  when (basicQuestionAnswerType == Just SingularOr) $ do
    let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers

 -- There are a ton of other validations that would be nice to have, like:
 -- - Are the correct answers formatted properly? (Should be answers separated
 --   by space-pipe-space, like `x | y | z`.)
 -- - Do any params not get substituted? In other words, are there any non-
 --   `Nothing` params that don't have a corresponding Mustache/ERB
 --   substitution?
 -- - Do any fields other than the question text "look like" question text?
 --   The easiest way to tell would be to look for Mustache/ERB tags.
 where
  isMultiple =
    basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice]

There also seems to be an issue here of indentation level. The code above is using special `where` indentation (1 space), but the comment is being indented with the `do` block (2 space). @stackptr what would you expect? this ```hs validateBasic :: forall m . (MonadThrow m, HasCallStack) => BasicQuestion -> m () validateBasic BasicQuestion {..} = do when (basicQuestionAnswerType == Just SingularOr) $ do let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers -- There are a ton of other validations that would be nice to have, like: -- - Are the correct answers formatted properly? (Should be answers separated -- by space-pipe-space, like `x | y | z`.) -- - Do any params not get substituted? In other words, are there any non- -- `Nothing` params that don't have a corresponding Mustache/ERB -- substitution? -- - Do any fields other than the question text "look like" question text? -- The easiest way to tell would be to look for Mustache/ERB tags. where isMultiple = basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice] ``` or ```hs validateBasic :: forall m . (MonadThrow m, HasCallStack) => BasicQuestion -> m () validateBasic BasicQuestion {..} = do when (basicQuestionAnswerType == Just SingularOr) $ do let invalidChars = findAll "!@#$%^&()" basicQuestionCorrectAnswers -- There are a ton of other validations that would be nice to have, like: -- - Are the correct answers formatted properly? (Should be answers separated -- by space-pipe-space, like `x | y | z`.) -- - Do any params not get substituted? In other words, are there any non- -- `Nothing` params that don't have a corresponding Mustache/ERB -- substitution? -- - Do any fields other than the question text "look like" question text? -- The easiest way to tell would be to look for Mustache/ERB tags. where isMultiple = basicQuestionAnswerType `elem` [Just MultipleAnswers, Just MultipleChoice] ```

lspitzner commented

2019-12-20 14:23:41 +01:00

(Migrated from github.com)

This is fixed on master. master on the online formatter was updated, so it works there already.

I think for the comment position nothing needs to be changed. The input is essentially already formatted, so brittany should not change anything about comment placement. Sometimes brittany changes comment indentation (e.g. if you have a comment in the middle of a do block and you switch from --indent 2 to --indent 4) but generally retaining comment position including indentation exactly is a feature.

This is fixed on master. `master` on the online formatter was updated, so it works there already. I think for the comment position nothing needs to be changed. The input is essentially already formatted, so brittany should not change anything about comment placement. _Sometimes_ brittany changes comment indentation (e.g. if you have a comment in the middle of a `do` block and you switch from `--indent 2` to `--indent 4`) but generally retaining comment position including indentation exactly is a feature.

Munksgaard commented

2020-02-11 08:42:53 +01:00

(Migrated from github.com)

This is fixed on master. master on the online formatter was updated, so it works there already.

Would it be possible to release a new version with the fix in it?

> This is fixed on master. `master` on the online formatter was updated, so it works there already. Would it be possible to release a new version with the fix in it?

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#263