Skip to content

Conversation

@james-d-mitchell
Copy link
Member

This PR updates AhoCorasick to be better in sync with libsemigroups, and some other minor fixes, including in the check_sync script.

Refactored argument parsing for better readability and removed unused --show-skipped option. Enhanced C++ comment stripping, added variable detection, and improved search patterns for functions and attributes in C++ files. Updated skip logic to use Console for output and improved match reporting for found and possibly found items.
Copy link
Collaborator

@Joseph-Edwards Joseph-Edwards left a comment

Choose a reason for hiding this comment

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

AhoCorasick changes look good to me, and the check sync stuff seems to work for me too once I had installed rich. Please could you add rich to the dev environment. After that, I'm happy to merge

def find_in_cpp(args, thing: str, fn: str, info: dict) -> None:
if _skip(args, thing, fn):
return
def _strip_cxx_comments(lines: list[str]) -> list[str]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it necessary to also remove comments of the following form:

/*
This is a comment
*/

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't ever use these (this is also complained about by cpplint I think), so I think the answer is "no"

@james-d-mitchell
Copy link
Member Author

I've added rich to the dev-environment.yml file, are we good to go? @Joseph-Edwards

@Joseph-Edwards Joseph-Edwards merged commit a3efc4c into libsemigroups:main Nov 4, 2025
13 checks passed
@james-d-mitchell james-d-mitchell deleted the sync-aho-corasick branch November 4, 2025 13:41
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