Guards lead to unreadable code #304

New Issue

2020-05-05T21:31:37+02:00

jrp2014 commented

2020-05-05 21:31:37 +02:00

(Migrated from github.com)

Brittany generally does a great job, but he following code is not rendered very prettily:

  91 usedImport dynflags action anns impPs@(L (RealSrcSpan locPs) declPs) (L (RealSrcSpan _) declRn, used, unused)
   92 
   93 -- unused applies to an explicit import list.  It will be null for a simple module
   94 -- imports
   95 
   96   | -- Do not remove `import M ()`
   97     Just (False, L _ []) <- ideclHiding declRn
   98   = (anns, [impPs])
   99   | -- Note [Do not warn about Prelude hiding]
  100     -- TODO: add ability to support custom prelude
  101     Just (True, L _ hides) <- ideclHiding declRn
  102   , not (null hides)
  103   , ideclImplicit declRn -- pRELUDE_NAME == unLoc (ideclName decl)
  104   = (anns, [impPs])
  105   | -- Nothing used
  106     null used
  107   = case action of

Putting the "=" under the "|" seems to be part of the cause. Putting the comments on the same line as the "|" rather than on a preceding line is another thing that could be improved.

(ormolu is better, but not much)

   92 usedImport dynflags action anns impPs@(L (RealSrcSpan locPs) declPs) (L (RealSrcSpan _) declRn, used, unused)
   93   -- unused applies to an explicit import list.  It will be null for a simple module
   94   -- import
   95 
   96   | -- Do not remove `import M ()`
   97     Just (False, L _ []) <- ideclHiding declRn =
   98     (anns, [impPs])
   99   | -- Note [Do not warn about Prelude hiding]
  100     -- TODO: add ability to support custom prelude
  101     Just (True, L _ hides) <- ideclHiding declRn,
  102     not (null hides),
  103     ideclImplicit declRn = -- pRELUDE_NAME == unLoc (ideclName decl)
  104     (anns, [impPs])
  105   | -- Nothing used
  106     null used =

Brittany generally does a great job, but he following code is not rendered very prettily: ``` 91 usedImport dynflags action anns impPs@(L (RealSrcSpan locPs) declPs) (L (RealSrcSpan _) declRn, used, unused) 92 93 -- unused applies to an explicit import list. It will be null for a simple module 94 -- imports 95 96 | -- Do not remove `import M ()` 97 Just (False, L _ []) <- ideclHiding declRn 98 = (anns, [impPs]) 99 | -- Note [Do not warn about Prelude hiding] 100 -- TODO: add ability to support custom prelude 101 Just (True, L _ hides) <- ideclHiding declRn 102 , not (null hides) 103 , ideclImplicit declRn -- pRELUDE_NAME == unLoc (ideclName decl) 104 = (anns, [impPs]) 105 | -- Nothing used 106 null used 107 = case action of ``` Putting the "=" under the "|" seems to be part of the cause. Putting the comments on the same line as the "|" rather than on a preceding line is another thing that could be improved. (ormolu is better, but not much) ``` 92 usedImport dynflags action anns impPs@(L (RealSrcSpan locPs) declPs) (L (RealSrcSpan _) declRn, used, unused) 93 -- unused applies to an explicit import list. It will be null for a simple module 94 -- import 95 96 | -- Do not remove `import M ()` 97 Just (False, L _ []) <- ideclHiding declRn = 98 (anns, [impPs]) 99 | -- Note [Do not warn about Prelude hiding] 100 -- TODO: add ability to support custom prelude 101 Just (True, L _ hides) <- ideclHiding declRn, 102 not (null hides), 103 ideclImplicit declRn = -- pRELUDE_NAME == unLoc (ideclName decl) 104 (anns, [impPs]) 105 | -- Nothing used 106 null used = ```

👍 2

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