Skip to content

chore: Fix hlint action and apply changes#105

Open
croyzor wants to merge 4 commits intomainfrom
cr/hlint-action
Open

chore: Fix hlint action and apply changes#105
croyzor wants to merge 4 commits intomainfrom
cr/hlint-action

Conversation

@croyzor
Copy link
Collaborator

@croyzor croyzor commented Mar 3, 2026

No description provided.

@croyzor croyzor changed the title Fix actions syntax for hlint chore(CI): Fix actions syntax for hlint Mar 3, 2026
@croyzor croyzor changed the title chore(CI): Fix actions syntax for hlint chore: Fix hlint action and apply changes Mar 3, 2026
@croyzor croyzor requested a review from acl-cqc March 3, 2026 14:13
@croyzor croyzor marked this pull request as ready for review March 3, 2026 14:13
Copy link
Collaborator

@acl-cqc acl-cqc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It dislikes $ inside brackets? Can we turn that off?

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)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I'm not sure I see this as an improvement

-- 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bit of a shame some kind of too-much-on-one-line thing doesn't kick in here really!

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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hang on - snake_camelCase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a good change but is that really result of a lint??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants