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

Allow filtering based on the node name #219

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Allow filtering based on the node name #219

merged 1 commit into from
Jun 4, 2024

Conversation

psss
Copy link
Collaborator

@psss psss commented Jan 22, 2024

Add support for filtering based on the node name by allowing plain literals without the key: name format.

@psss psss added this to the 1.4 milestone Jan 22, 2024
@psss psss linked an issue Jan 22, 2024 that may be closed by this pull request
@psss
Copy link
Collaborator Author

psss commented Jan 25, 2024

Summary from the hacking session: Use no key for search by fmf node name. Example:

/tests/core & tag:quick

So we don't have to invent/decide any name ;-)

@lukaszachy
Copy link
Collaborator

/packit test

@lukaszachy
Copy link
Collaborator

@psss Is the "/tests/core & tag:quick" possible with current PR? It doesn't seem so.
Also adding a unit test and similar example do the pydoc fmf.filter will be useful.

@The-Mule
Copy link

The-Mule commented May 2, 2024

This actually brings some issues. See, right now fmf filter is unable parse regular expressions that are quite common and that I assume people will likely be using (there is already a misplaced issue in tmt project for this. This problem is solvable[*] under the assumption that keys are always used. But once this PR is merged this assumption is gone.

[*] Using a positive lookahead assertion as follows:

# At least one clause must be true
    return any([check_clause(clause)
                for clause in re.split(r"\s*\|\s*(?=[^|]*:)", filter)])

Once you have | in a regexp the only way to distinguish it from proper disjunction is to to check that some key follows it. If not then it must be a part of value of the last key. Same approach can be applied to identifying a proper conjunction. With that you can already use pretty powerful regular expressions as values (you still need to avoid using both | and : in you regexp, but probability of that is fairly low IMO). Allowing not to have a key will break this logic.

@lukaszachy
Copy link
Collaborator

Once you have | in a regexp the only way to distinguish it from proper disjunction is to to check that some key follows it. If not then it must be a part of value of the last key. Same approach can be applied to identifying a proper conjunction. With that you can already use pretty powerful regular expressions as values (you still need to avoid using both | and : in you regexp, but probability of that is fairly low IMO). Allowing not to have a key will break this logic.

With escaping it is clear whether | is part of filter syntax or regexp (\|). Escaping of ':' is already supported (driven by colon used in module nsv strings used as component value), escaping '&' and '|' are planned in this release.

@psss psss force-pushed the fmf-name-in-filter branch from 5433e36 to eca5736 Compare June 3, 2024 12:17
@psss psss requested a review from martinhoyer June 3, 2024 12:17
Add support for filtering based on the node name by allowing plain
literals without the `key: name` format.
@psss psss force-pushed the fmf-name-in-filter branch from eca5736 to 4f88531 Compare June 3, 2024 12:36
@psss psss requested a review from lukaszachy June 3, 2024 12:37
@psss psss mentioned this pull request Jun 3, 2024
@psss
Copy link
Collaborator Author

psss commented Jun 3, 2024

@psss Is the "/tests/core & tag:quick" possible with current PR? It doesn't seem so. Also adding a unit test and similar example do the pydoc fmf.filter will be useful.

Implemented and documented. @lukaszachy, please review.

@psss psss self-assigned this Jun 4, 2024
@psss psss merged commit 4f88531 into main Jun 4, 2024
10 checks passed
@psss psss deleted the fmf-name-in-filter branch June 4, 2024 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to use--filter for name
4 participants