chore: Fix hlint action and apply changes#105
Conversation
croyzor
changed the title
croyzor
changed the title
acl-cqc
left a comment
acl-cqc
left a comment
There was a problem hiding this comment.
Generally - great :-) ❤️
A few "can we turn this off"s, I admit I can see it may be hard to both "remove redundant brackets" and "allow brackets that make precedence clear"....
And not quite sure what's going on with the camelCase.
Do you wanna have a quick look through and see if there's anything you can tweak, merge main in and then I'll have a last look?
| where | ||
| helper :: VEnv -> forall a. CheckingSig a -> Checking (Maybe a) | ||
| helper avail (VLup x) | j@(Just new) <- M.lookup x avail = | ||
| (req $ AddCapture n (x,new)) >> (pure $ Just j) |
There was a problem hiding this comment.
It dislikes $ inside brackets? Can we turn that off?
There was a problem hiding this comment.
Only when it means we can get rid of the outer brackets
| InEnd inport -> case M.lookup inport (dynamicSet ctx) of | ||
| Just fc -> track ("Replace " ++ show end ++ " with " ++ show newDynamics) $ | ||
| M.union | ||
| (M.fromList (zip newDynamics (repeat fc))) |
There was a problem hiding this comment.
Similarly, I'm not sure I see this as an improvement
There was a problem hiding this comment.
I think this is good, zip is dangerous to begin with; it's nicer to use one thing many times than make an infinite list of it and pull from that
| -- being replaced, although this is not enforced. | ||
| splice :: forall m n. (Ord n, Ord m) => n -> HugrGraph m -> (m -> n) -> State (HugrGraph n) () | ||
| splice hole add non_root_k = modify $ \host -> case (M.lookup hole (nodes host) >>= isHole) of | ||
| splice hole add non_root_k = modify $ \host -> case M.lookup hole (nodes host) >>= isHole of |
There was a problem hiding this comment.
bit of a shame some kind of too-much-on-one-line thing doesn't kick in here really!
There was a problem hiding this comment.
Yeah I think this isn't offered in hlint because it's operating on the AST
| edges_out = disj_union (edges_out host) new_edges_out, | ||
| first_children = disj_union (first_children host) | ||
| (M.mapKeys k $ M.map (k <$>) $ first_children add) | ||
| edgesIn = disj_union (edgesIn host) new_edgesIn, |
There was a problem hiding this comment.
hang on - snake_camelCase?
There was a problem hiding this comment.
that can't be right!
| ,"repeated_app" -- missing coercions, https://github.com/quantinuum-dev/brat/issues/413 | ||
| ,"thunks"] | ||
| ) ++ ["test/compilation/closures.brat"] -- fails to compile but still spits out some JSON (not whole Hugr) | ||
| ,"thunks"] ++ ["test/compilation/closures.brat"] -- fails to compile but still spits out some JSON (not whole Hugr) |
There was a problem hiding this comment.
can you
,"thunks"
] ++ ["test/compilation/closures.brat"]
?
| Right res -> assertFailure ("Computation produced result " ++ show res ++ " when should have Thrown") | ||
| Left err -> let shown = showError err in | ||
| if isInfixOf needle shown then pure () else assertFailure ("Unexpected error " ++ shown) | ||
| if needle `isInfixOf` shown then pure () else assertFailure ("Unexpected error " ++ shown) |
There was a problem hiding this comment.
Seems a good change but is that really result of a lint??
|
The "Redundant |
2 participants
No description provided.