Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify hyrolo match count #500

Merged
merged 2 commits into from
Mar 31, 2024
Merged

Verify hyrolo match count #500

merged 2 commits into from
Mar 31, 2024

Conversation

matsl
Copy link
Collaborator

@matsl matsl commented Mar 30, 2024

What

Verify hyrolo match count.

Why

Hyrolo match regexps under different options. This is a start
verifying the count result using either match all or just match
headlines. Both types of matches looks like they fail.

The first search for all matches returns 2 but in HyRolo all
three lines containing "match" are highlighted. The second
search for just searching in headlines does not find any match.

Sharing for information. I tried to debug this today but with no
success.

Marked as expected to fail for not breaking the CI.

Note

Can I have I misunderstood what headline means in this
context? I'm assuming that in org mode it is the first line
/following a star.

@matsl matsl requested a review from rswgnu March 30, 2024 22:56
@rswgnu
Copy link
Owner

rswgnu commented Mar 31, 2024

The HyRolo count values are how many records matched the search pattern, not how many matches exist within the records, i.e. records can have more than one match to the pattern. So the correct value in your test is 2. That is what you should get.

@matsl
Copy link
Collaborator Author

matsl commented Mar 31, 2024

The HyRolo count values are how many records matched the search pattern, not how many matches exist within the records, i.e. records can have more than one match to the pattern. So the correct value in your test is 2. That is what you should get.

Aha. I see. A third option on how it should be interpreted. 😄 That is good news. Just a misunderstanding on my part then. I'll update the test case and see what more tests I can construct.

What do you think of making the docs clearer on this part so even I can understand? 😆

@matsl matsl force-pushed the add-test-for-hyrolo-headline-only branch from ebad0d7 to 07891ea Compare March 31, 2024 09:54
@matsl
Copy link
Collaborator Author

matsl commented Mar 31, 2024

@rswgnu

I'll update the test case and see what more tests I can construct.

The tests are now updated and I have added some more verification. Count entries with only headerline match, count entries with no headerline match and no match at all. I can't get the numbers to line up so the test case is still marked as an expected failure.

Can it be further misunderstand on my part of what different values on the parameters MAX-MATCHES and HEADLINE-ONLY really means?

@rswgnu
Copy link
Owner

rswgnu commented Mar 31, 2024 via email

@matsl
Copy link
Collaborator Author

matsl commented Mar 31, 2024

Well, I did clarify the docs with my pushes last night.

😄 That is like hitting a running target. To hard for me!

@matsl matsl force-pushed the add-test-for-hyrolo-headline-only branch from 07891ea to 37e6150 Compare March 31, 2024 17:24
@matsl matsl merged commit 8adaf75 into master Mar 31, 2024
5 checks passed
@matsl matsl deleted the add-test-for-hyrolo-headline-only branch March 31, 2024 17:26
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