wbolster
Use tree sitter for getting the node id (aka test id) for the function and class at point
The master branch uses the function python-pytest--current-defun for getting the test id of the function at point or class at point and it supports at most 2 parts. The changes in this pull request adds two functions: python-pytest--node-id-def-at-point (link) and python-pytest--node-id-class-at-point (link) which get the path for the def or class at point for an arbitrary number of nested classes, these two functions use tree-sitter. See the file tests/test-python-helpers.el (link to file) for examples on how these functions behave. By having two functions for different purposes, the user has more control over what to execute (e.g. (s)he could execute a Test class when the point is on a function).
Please let me know what you think of these changes, so that I can improve this pull request to suit more use cases.
I just noticed that I have used both terms "path" and "node id" in function names and docstrings for referring to the same thing. I think it would be better to use the same terminology used in the pytest documentation, that is to use "node-id" instead of "path"
Retrieved from https://docs.pytest.org/en/8.3.x/example/markers.html#node-id
Node IDs are of the form module.py::class::method or module.py::function. Node IDs control which tests are collected, so module.py::class will select all test methods on the class. Nodes are also created for each parameter of a parametrized fixture or test, so selecting a parametrized test must include the parameter value, e.g. module.py::function[param].
Retrieved from https://docs.pytest.org/en/8.3.x/example/markers.html#selecting-tests-based-on-their-node-id
You can provide one or more node IDs as positional arguments to select only specified tests. This makes it easy to select tests based on their module, class, method, or function name:
Give me a few minutes to make another commit.
With a caveat that I haven't read all code changes yet, I'd like to rise one point.
I think that the change is great, and I believe using treesit is the way to go in a future. However, the package (python-pytest) is marked as Emacs-24 compatible and, IIRC, treesit has only been available since Emacs-29. Would this change work in older Emacs? Perhaps some compatibility layer would be needed (I doubt that compat has treesit, but maybe worth checking)? Alternatively, perhaps the minimum version required should be updated?
same concerns here!
i'm all for using more modern and more reliable ways to do syntactical heavy lifting, but i don't want to break this package for people who are using reasonably modern operating systems (e.g. ubuntu lts).
that said, emacs 24 is essentially a random number that was perhaps relevant at some point for some compat reason, but that version is from the 2012–2015 era (release history) and i don't care at all about ancient software. (if you do, good for you, but then also use this package from that time… which didn't exist 🙃)
Moving discussion about minimum required Emacs back to main thread.
... but i don't want to break this package for people who are using reasonably modern operating systems (e.g. ubuntu lts).
I agree. I didn't consider versions that don't have tree-sitter when I did the previous commits, but now I think we should consider them.
I think I could add a conditional in the transient so that it displays the new functions if the user is using GNU Emacs that supports tree-sitter (i.e. >= 29). Users that don't have tree-sitter support would continue using
python-pytest--current-defun, and users that have tree-sitter would usepython-pytest--node-id-def-at-pointandpython-pytest--node-id-class-at-point.
I think this (a compatibility layer) was something I have been trying to suggest in my original comment. The only nit pick would be - as I was once told - the recommended solution is to use some predicate (i.e., one of functionp/packagep/featurep) in such an if block. IMHO an implementation like that is slightly harder to maintain (than an explicit version check) in longer period. Especially when minimum supported Emacs version is meaningful: it requires a mental mapping between supported functionality and Emacs versions. I tend to add comments with minimal version in places like that.
Last but not least, you’d probably want to add (require treesit nil t) stanza to the beginning of the main package file.
In the recently introduced commits ...
- I removed the
(declare (obsoletestatements because as I mentioned previously, I agree that we should not break this package for users that don't use treesit. - I added the suffix
-treesitto those functions that use treesit features. This will help us maintain the code since now we have two functions that does similar tasks but use different algorithms. - I have added a defcustom to control whether functions that use treesit should be displayed in the transient. See https://github.com/rodrigomorales1/emacs-python-pytest/blob/a104cf85b6acf1e3f37f5faf4a9a9060d92b305e/python-pytest.el#L130-L138 and https://github.com/rodrigomorales1/emacs-python-pytest/blob/a104cf85b6acf1e3f37f5faf4a9a9060d92b305e/python-pytest.el#L191-L194
@pkryger @wbolster Please let me know what you think.
In the latest commit, I improved *-treesit functions so that they can correctly handle narrowed buffers. I also added tests when the buffer is narrowed.