Use tree sitter for getting the node id (aka test id) for the function and class at point#75
Conversation
We rename the parameter to improve the readability of the code because the function python-pytest--run will be used for executing specific functions and classes in future commits and each function or class is identified by their node id. The term "node id" is used in the official pytest documentation: https://docs.pytest.org/en/7.1.x/how-to/usage.html#nodeids
We need to create a new version because in future commits we will mark a function as obsolete since this new version.
The new functions that should be used python-pytest--path-def-at-point are python-pytest--path-class-at-point. python-pytest--current-defun could obtain the identifiers for functions and classes. If the current point was inside a function, there was no way to get the test id of the current class. By using python-pytest--path-def-at-point and python-pytest--path-class-at-point, there is no such limitation.
The new functions that should be used are python-pytest-run-def-at-point and python-pytest-run-class-at-point.
|
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
Retrieved from https://docs.pytest.org/en/8.3.x/example/markers.html#selecting-tests-based-on-their-node-id
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 |
|
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.
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 Last but not least, you’d probably want to add |
In a recent previous commit, python-pytest--current-defun had been marked as obsolete and all function calls have been replaced in favour of python-pytest--node-id-def-at-point. However, we should continue supporting python-pytest--current-defun so that users that use a GNU Emacs version that don't have tree-sitter support can continue using this package. See comment: wbolster#75 (comment) To make it clear, Emacs users that have tree-sitter support should use python-pytest--node-id-def-at-point and python-pytest--node-id-class-at-point. Emacs users that don't have tree-sitter support should use python-pytest--current-defun and those functions that call that function (i.e. python-pytest-function and python-pytest-run-def-at-point-dwim).
In a recent previous commit, we have added 2 functions: python-pytest--node-id-def-at-point-treesit and python-pytest--node-id-class-at-point-treesit. These two function names clearly explain that they either get the node id of the def at or class at point. The name python-pytest--current-defun can be confusing for some readers because the reader might think that it can only get the def at point, not the class at point. The new name python-pytest--node-id-def-or-class-at-point makes it clear that it can get both the def or class at point.
The functions python-pytest-function and python-pytest-run-def-at-point-dwim call python-pytest--node-id-def-or-class-at-point. We rename those 2 functions for better readibility.
|
In the recently introduced commits ...
|
| (set-default symbol value) | ||
| value)))) | ||
|
|
||
| (defcustom python-pytest-use-treesit nil |
There was a problem hiding this comment.
How about
| (defcustom python-pytest-use-treesit nil | |
| (defcustom python-pytest-use-treesit (featurep 'treesit) |
There was a problem hiding this comment.
is there any reason to NOT use treesitter even though it's available? i'd rather have less configuration and more ‘dwim’ behaviour, and not having this customizable at all would have my preference.
There was a problem hiding this comment.
is there any reason to NOT use treesitter even though it's available?
@wbolster There are some differences between the functions in main branch and the functions that use treesit. Users that still prefer the behavior in main branch could set the variable python-pytest-use-treesit to nil. I suggest that we have a custom variable to provide users more control on how the package works.
Difference 1: Depths of classes
In main branch, the functions python-pytest-function (renamed to python-pytest-run-def-or-class-at-point in a commit in this pull request) and python-pytest-function-dwim (renamed to python-pytest-run-def-or-class-at-point-dwim in a previous commit in this pull request) both use python-pytest--current-defun (renamed to python-pytest--node-id-def-or-class-at-point in a previous commit in this pull request).
In main branch, python-pytest--current-defun gets the current function or class at point up to a second depth. The new function python-pytest--node-id-class-at-point-treesit get the class at point for an arbitrary depth. The new function python-pytest--node-id-def-at-point-treesit gets the def at point for the outermost function (because pytest doesn't allow grouping tests using functions inside functions, it only allows using classes inside classes). See example below.
class ClassDepthOne:
class ClassDepthTwo:
class ClassDepthThree:
def print_foo(self):
def print_bar():<point>
print("foo")
ELISP> (python-pytest--current-defun)
"ClassDepthOne::ClassDepthTwo"
ELISP> (python-pytest--node-id-class-at-point-treesit)
"ClassDepthOne::ClassDepthTwo::ClassDepthThree"
ELISP> (python-pytest--node-id-def-at-point-treesit)
"ClassDepthOne::ClassDepthTwo::ClassDepthThree::print_foo"
There are some differences between the functions with the suffix -treesit and the functions in main branch. Users that still prefer the behavior in main branch, could use
Difference 2: dwim behavior
In main branch, the function python-pytest-function-dwim search for a test file and for test names when the current file is not a test file (see below). There is no function that use treesit and implement such behavior.
emacs-python-pytest/python-pytest.el
Lines 293 to 294 in bfcd288
|
In the latest commit, I improved |
|
lgtm 🚀 |
|
merged, thanks @rodrigomorales1 and @pkryger! |
These two commands were renamed upstream and pulled in in 1fa1eba: - python-pytest-function-dwim -> python-pytest-run-def-or-class-at-point - python-pytest-function-dwim -> python-pytest-run-def-or-class-at-point-dwim Amend: 1fa1eba Ref: wbolster/emacs-python-pytest#75
The
masterbranch uses the functionpython-pytest--current-defunfor 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) andpython-pytest--node-id-class-at-point(link) which get the path for thedeforclassat point for an arbitrary number of nested classes, these two functions use tree-sitter. See the filetests/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.