Feature suggestion: Add support for rich comments

[jonasseglare]

This pull request adds code that recognizes blocks starting with if False: as code cells. This is an attempt at having something similar to rich comments commonly used in Clojure. I currently use this code in my Emacs setup and find it very convenient. But you do not have to accept this PR to your repo, it is just a suggestion in case others could also find use for this code.

The idea is that if False: prevents the code inside from being evaluated, similar to the (comment ...) macro in Clojure. In Clojure, it is common to add such comments wrapping code at the end of modules for examples and experiments and editors can be configured to execute the code in those comments when the cursor is placed in the comment. With the code in this pull request, if False-blocks are recognized as cells and the code wrapped inside those blocks is evaluated when we send the cell to the REPL. But the code is not executed if the module is run normally, e.g. by importing it from another module.

[twmr]

Hi! Thx a lot for your contribution!

if False-blocks are recognized as cells and the code wrapped inside those blocks is evaluated when we send the cell to the REPL

Very nice! I'll test this change in the coming days and will then probably merge it. Thx!

[jonasseglare]

I have doubts about the naming of this function and the refactoring I did related to it. Essentially, I refactored the function python-cell-beginning-of-cell into more functions to have better control of where at the beginning the cursor is placed:

• When I navigate to a cell, I want the cursor where the actual code starts, that is after the cellbreak string (if False... or ##....)
• When I send a cell to the interpreter, I want the cursor to be at the very beginning of the cell, including the cellbreak string.

To accomplish this, the function python-cell-beginning-of-cell--sub accepts a function as argument that will put the cursor in the right place in case a cellbreak is detected. Either we pass python-cell-beginning-of-matched-cellbreak or python-cell-end-of-matched-cellbreak to that function. The original python-cell-beginning-of-cell function passes in the former.

Is there a better way to structure this code? What about naming? Feel free to comment on this and I will address it.

[jonasseglare]

I wonder if the code would be more clear if I instead had a function something like

(defun python-cell-detect-beginning-of-cell ()
  (end-of-line)
    (if (re-search-backward python-cell-cellbreak-regexp nil t)
        t
      (progn (goto-char (point-min)) nil)))

that returns a truthy value in case the beginning of the cell is detected. And then, in that case, the caller of that function can move the cursor:

(when (python-cell-detect-beginning-of-cell)
   (python-cell-end-of-matched-cellbreak))

This might make the code more readable but I am really not sure.

[twmr]

• When I send a cell to the interpreter, I want the cursor to be at the very beginning of the cell, including the cellbreak string.

So you want to send the if False string to the python shell s.t. the body of the if False statement is not executed by the python shell? Why that?

[jonasseglare]

Because in the function python-cell-shell-send-cell we call python-cell-preprocess-cell that will replace if False: by if True:. Because of this replacement, the body of the cell will be run.

For instance, if we have a cell

if False: # load the dataset
    ds = load_dataset("data.csv")

the string sent to Python becomes

if True: # load the dataset
    ds = load_dataset("data.csv")

Is there a better way to accomplish the effect of having cells with if False: executed?

[twmr]

Thx for the explanation! Now it starts to make sense.

Is there a better way to accomplish the effect of having cells with if False: executed?

I don't think so.

BTW It would be nice if there were a way to detect the end of the "rich-comment". I guess that the recently added support for tree-sitter in emacs-29 could be used for that.

[jonasseglare]

Yes, detecting the end of rich-comments would mean that the following code

if False:
    dataset = load_dataset("validation.py")

results = algorithm.evaluate(dataset);

would actually become two cells, right? The two cells being

if False:
    dataset = load_dataset("validation.py")

and

results = algorithm.evaluate(dataset);

I am not sure how useful that would be and it would probably be tricky to get it work. So maybe it is something to consider outside of the scope of this PR.

[jonasseglare]

Thanks @Thisch for python-cell.el, I have been using it for years. Feel free to make any changes you like to the code. Or you can ask me to change it, whatever you prefer.

[jonasseglare]

We could allow any number of spaces between if and False but that would make the logic inside python-cell-preprocess-cell more complicated. And I don't know if there would be a need for any other code formatting than if False: so maybe it is good enough to only detect exactly this string.

[twmr]

I think it suffices to hard code "if False:" as you did. Why do you have to use group-n in the rx macro, isn't the following sufficient?

  (rx line-start (* space)
      (group (and (or (and "#" (or (and "#" space (* (not (any "\n"))))
                                   (and " <" (or "codecell" "markdowncell") ">")))
                      "if False:")
                  line-end)))

[jonasseglare]

That is a good point. I initially wrote something similar to that code. But I think that this version with only one group is going to detect also rich comments preceded by space, e.g. if False:. I wanted to prohibit that, because evaluating that cell would probably be invalid syntax unless we remove the indentation before sending it to the Python interpreter. If we were to support indentation in front of if False:, we could maybe have cells defined inside other code such as functions, but I think that would complicate things and not be very useful, so I disallow it.~~

Edited: Hmm, I still have doubts and have not settled. Do we want to detect rich comments if they are indented?

The (group-n 1 ...) is needed over (group ...) in order for the code that I wrote to work properly.