unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
@ 2022-07-18 22:20 Dima Kogan
  2022-07-23  7:58 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 21+ messages in thread
From: Dima Kogan @ 2022-07-18 22:20 UTC (permalink / raw)
  To: 56635

Hi. I'm using hide-show mode to visually collapse blocks while coding.
This is very helpful, and works great in C-like modes. hide-show mode
currently supports Python in a limited fashion: only def: and class:
blocks are hidden. Other kinds of blocks (if, else, for, while, ...) are
not supported. A simple patch that mostly adds support for all other
kinds of blocks is:

  diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
  index 1c99937c4b9..b63d2bc0690 100644
  --- a/lisp/progmodes/python.el
  +++ b/lisp/progmodes/python.el
  @@ -4819,7 +4819,7 @@ python-describe-at-point
   (defun python-hideshow-forward-sexp-function (_arg)
     "Python specific `forward-sexp' function for `hs-minor-mode'.
   Argument ARG is ignored."
  -  (python-nav-end-of-defun)
  +  (python-nav-end-of-block)
     (unless (python-info-current-line-empty-p)
       (backward-char)))

  @@ -5766,8 +5766,8 @@ python-mode

     (add-to-list
      'hs-special-modes-alist
  -   '(python-mode
  -     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
  +   `(python-mode
  +     ,(python-rx block-start)
        ;; Use the empty string as end regexp so it doesn't default to
        ;; "\\s)".  This way parens at end of defun are properly hidden.
        ""

I say "mostly" because this isn't doable with regular expressions, as is
required by hide-show mode. So this patch can confuse the parser: some
bits of code will look like they begin a block, while in reality they do
not. hide-show would then collapse lines that it should ignore. Some
Python code that shows this failure:

  if 0:

      aaa
      l = [ i for i in range(5) \
            if i < 3 ]
      ccc
      abc = o.match(1,2,3)
      ddd

Here the "for" and "if" in the list comprehension both trigger the block
hiding. And the "match" function call triggers it too. python-mode has
functions to move between blocks (python-nav-...), but they're not
completely regex-based, so they can't be baked into hide-show.
Suggestions?

Obvious options:

  1. Leave the code as it is, don't support other blocks

  2. Take this patch, with the understanding that it'll collapse some
     stuff that it shouldn't

  3. Patch hide-show to potentially accept non-regex logic

I've been dogfooding the patch for a few days now, and it's behaving 99%
properly for me, so I'm inclined to suggest option 2. Making hide-show
smarter (option 3) doesn't feel worth the effort to fix THIS problem.

Thanks!





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-18 22:20 bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks Dima Kogan
@ 2022-07-23  7:58 ` Lars Ingebrigtsen
  2022-07-23 15:51   ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-07-23  7:58 UTC (permalink / raw)
  To: Dima Kogan; +Cc: kobarity, 56635

Dima Kogan <dima@secretsauce.net> writes:

> Obvious options:
>
>   1. Leave the code as it is, don't support other blocks
>
>   2. Take this patch, with the understanding that it'll collapse some
>      stuff that it shouldn't
>
>   3. Patch hide-show to potentially accept non-regex logic
>
> I've been dogfooding the patch for a few days now, and it's behaving 99%
> properly for me, so I'm inclined to suggest option 2. Making hide-show
> smarter (option 3) doesn't feel worth the effort to fix THIS problem.

Hm; don't know either.  Kobarity's done some work in related areas in
python-mode lately, and perhaps has some comments, so added to the CCs.






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-23  7:58 ` Lars Ingebrigtsen
@ 2022-07-23 15:51   ` kobarity
  2022-07-23 20:41     ` Dima Kogan
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-07-23 15:51 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Dima Kogan, 56635

Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Hm; don't know either.  Kobarity's done some work in related areas in
> python-mode lately, and perhaps has some comments, so added to the CCs.

Thank you for letting me know about this issue.
It would be nice if Python hideshow works for blocks.  May I have some
time to think about it?

Regards,





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-23 15:51   ` kobarity
@ 2022-07-23 20:41     ` Dima Kogan
  2022-07-24  0:57       ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Dima Kogan @ 2022-07-23 20:41 UTC (permalink / raw)
  To: kobarity; +Cc: Lars Ingebrigtsen, 56635

Thanks for taking the time to look at this. After a big more dogfooding,
I've a small tweak to the first piece of the patch:

  diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
  index 1c99937c4b9..2b2b0900bee 100644
  --- a/lisp/progmodes/python.el
  +++ b/lisp/progmodes/python.el
  @@ -4819,9 +4819,7 @@ python-describe-at-point
   (defun python-hideshow-forward-sexp-function (_arg)
     "Python specific `forward-sexp' function for `hs-minor-mode'.
   Argument ARG is ignored."
  -  (python-nav-end-of-defun)
  -  (unless (python-info-current-line-empty-p)
  -    (backward-char)))
  +  (python-nav-end-of-block))

   \f
   ;;; Imenu

So (python-hideshow-forward-sexp-function) should call
(python-nav-end-of-block) and do nothing else. I think the extra
(backward-char) stuff was intended to handle the closing brace of the
block, which doesn't exist in Python. If you leave the (backward-char)
stuff there, the collapsed blocks still display the last character in
the block, which in Python has no special syntactical meaning.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-23 20:41     ` Dima Kogan
@ 2022-07-24  0:57       ` kobarity
  2022-07-31 10:18         ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-07-24  0:57 UTC (permalink / raw)
  To: Dima Kogan; +Cc: Lars Ingebrigtsen, 56635

Hi Dima,

Dima Kogan <dima@secretsauce.net> wrote:
> So (python-hideshow-forward-sexp-function) should call
> (python-nav-end-of-block) and do nothing else. I think the extra
> (backward-char) stuff was intended to handle the closing brace of the
> block, which doesn't exist in Python. If you leave the (backward-char)
> stuff there, the collapsed blocks still display the last character in
> the block, which in Python has no special syntactical meaning.

This is one point I was thinking of.  The first patch failed to pass
ERTs python-hideshow-hide-levels-1 and python-hideshow-hide-levels-2.
The second patch still fails with python-hideshow-hide-levels-1, but I
think it's OK to fix the ERT:

diff --git a/test/lisp/progmodes/python-tests.el
b/test/lisp/progmodes/python-tests.el
index 3b10bde23b..8cb2aa84d0 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5828,8 +5828,11 @@ python-hideshow-hide-levels-1
 class SomeClass:

     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))

 (ert-deftest python-hideshow-hide-levels-2 ()


It would be better to add some ERTs for blocks.

Another point I am thinking of is to improve the regex.  As
hs-special-modes-alist has a support of the form (COMPLEX-START
MDATA-SELECTOR), I tried with the code below:

  (add-to-list
   'hs-special-modes-alist
   `(python-mode
     (,(python-rx (seq (or (seq buffer-start (? ?\n)) (seq (not ?\\) ?\n))
                       (* space) (group block-start))) 1)
     ;; Use the empty string as end regexp so it doesn't default to
     ;; "\\s)".  This way parens at end of defun are properly hidden.
     ""
     "#"
     python-hideshow-forward-sexp-function
     nil))

This code solves many problems of `hs-hide-block' mentioned in the
first mail, but it breaks `hs-hide-level'.  It seems that current
MDATA-SELECTOR support is limited and needs some fixes if we want to
do something like above.  I will look into this a little bit more.

Regards,





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-24  0:57       ` kobarity
@ 2022-07-31 10:18         ` kobarity
  2022-07-31 19:04           ` Dima Kogan
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-07-31 10:18 UTC (permalink / raw)
  To: Dima Kogan; +Cc: Lars Ingebrigtsen, 56635

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

I wrote:
> This code solves many problems of `hs-hide-block' mentioned in the
> first mail, but it breaks `hs-hide-level'.  It seems that current
> MDATA-SELECTOR support is limited and needs some fixes if we want to
> do something like above.  I will look into this a little bit more.

There are some problems in this approach.  The biggest one is the way
of processing in `hs-hide-level-recursive'.  It basically repeats:

1. hs-hide-block-at-point
2. forward-comment
3. re-search-forward hs-block-start-regexp

Let me explain with the following code:

#+begin_src python
class C:
    def a():
        pass

    def b():
        pass
#+end_src

Let's assume that point is at the beginning of the line "def a():".
The first step hides the method "a" and moves the point to the end of the
block.  The second step moves the point to the position "d" of "def b():",
not the beginning of the line.  Therefore, if hs-block-start-regexp
contains rx line-start, it fails to match "def b():".

Although this can be solved by adding a flag or a mode specific
forward-comment function to hs-special-modes-alist, I'm not sure if it
is the right approach.

So I think the following setting is best at this moment.

#+begin_src elisp
  (add-to-list
   'hs-special-modes-alist
   `(python-mode
     ,(python-rx (seq (* (syntax ?-)) block-start))
     ;; Use the empty string as end regexp so it doesn't default to
     ;; "\\s)".  This way parens at end of defun are properly hidden.
     ""
     "#"
     python-hideshow-forward-sexp-function
     nil))
#+end_src

(* (syntax ?-)) corresponds to "\\s-*" of the current regexp.  It
allows the point to be located before the block start symbols.

Another idea to improve the behavior of hiding a block is to provide a
wrapper function of `hs-hide-block' as follows and remap the key
bindings to this function like `python-mark-defun'.  With this
approach, C-c @ C-d and C-c @ C-h work as expected with the example
shown in the first mail.

#+begin_src elisp
(defun python-hideshow-hide-block (&optional end)
  "Python specific `hs-hide-block' function for `hs-minor-mode'.
Argument END is passed to `hs-hide-block'."
  (interactive "P")
  (python-nav-beginning-of-block)
  (hs-hide-block end))
#+end_src

Attached is a PoC level patch of this approach.  What do you think of
this approach?

Regards,

[-- Attachment #2: 56635-poc.patch --]
[-- Type: application/octet-stream, Size: 2082 bytes --]

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index b8fc7d4c54..ca8bff9a6e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -303,6 +303,8 @@ python-mode-map
     (define-key map "\C-c\C-v" #'python-check)
     (define-key map "\C-c\C-f" #'python-eldoc-at-point)
     (define-key map "\C-c\C-d" #'python-describe-at-point)
+    ;; Hideshow
+    (define-key map [remap hs-hide-block] #'python-hideshow-hide-block)
     ;; Utilities
     (substitute-key-definition #'complete-symbol #'completion-at-point
                                map global-map)
@@ -4825,9 +4827,16 @@ python-describe-at-point
 (defun python-hideshow-forward-sexp-function (_arg)
   "Python specific `forward-sexp' function for `hs-minor-mode'.
 Argument ARG is ignored."
-  (python-nav-end-of-defun)
-  (unless (python-info-current-line-empty-p)
-    (backward-char)))
+  (python-nav-end-of-block))
+
+(declare-function hs-hide-block "hideshow")
+
+(defun python-hideshow-hide-block (&optional end)
+  "Python specific `hs-hide-block' function for `hs-minor-mode'.
+Argument END is passed to `hs-hide-block'."
+  (interactive "P")
+  (python-nav-beginning-of-block)
+  (hs-hide-block end))
 
 \f
 ;;; Imenu
@@ -5773,8 +5782,8 @@ python-mode
 
   (add-to-list
    'hs-special-modes-alist
-   '(python-mode
-     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
+   `(python-mode
+     ,(python-rx (seq (* (syntax ?-)) block-start))
      ;; Use the empty string as end regexp so it doesn't default to
      ;; "\\s)".  This way parens at end of defun are properly hidden.
      ""
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 6f2ad87f81..bc56abaa2a 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5857,8 +5857,11 @@ python-hideshow-hide-levels-1
 class SomeClass:
 
     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))
 
 (ert-deftest python-hideshow-hide-levels-2 ()

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-31 10:18         ` kobarity
@ 2022-07-31 19:04           ` Dima Kogan
  2022-08-07 14:37             ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Dima Kogan @ 2022-07-31 19:04 UTC (permalink / raw)
  To: kobarity; +Cc: Lars Ingebrigtsen, 56635

Hi. Thanks for looking at this.

I do agree that the best thing to do is to use functions instead of pure
regexes here. Can we hook that into hs-special-modes-alist instead of
doing something specific in python-mode? I'm imagining extending the
meaning of the fields in hs-special-modes-alist, where we'd do something
different, depending on if some element is a string or not. Maybe that
would make it easier to extend the hideshow support for other languages?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-07-31 19:04           ` Dima Kogan
@ 2022-08-07 14:37             ` kobarity
  2022-08-08 15:16               ` Augusto Stoffel
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-08-07 14:37 UTC (permalink / raw)
  To: Dima Kogan; +Cc: Lars Ingebrigtsen, 56635

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

Hi,

Dima Kogan <dima@secretsauce.net> wrote:
> I do agree that the best thing to do is to use functions instead of pure
> regexes here. Can we hook that into hs-special-modes-alist instead of
> doing something specific in python-mode? I'm imagining extending the
> meaning of the fields in hs-special-modes-alist, where we'd do something
> different, depending on if some element is a string or not. Maybe that
> would make it easier to extend the hideshow support for other languages?

If we are to extend hs-special-modes-alist, I think it's better to add
functions to find/check blocks. Maybe this is the third option
mentioned in the first mail.  For that, I added three functions to
hs-special-modes-alist:

- hs-find-block-beginning-func
- hs-find-next-block-func
- hs-looking-at-block-start-p-func

Attached is a prototype patch to achieve this.

Regards,

[-- Attachment #2: 56635-prototype.patch --]
[-- Type: application/octet-stream, Size: 9519 bytes --]

diff --git a/lisp/progmodes/hideshow.el b/lisp/progmodes/hideshow.el
index f574ec84fb..6eef2c3271 100644
--- a/lisp/progmodes/hideshow.el
+++ b/lisp/progmodes/hideshow.el
@@ -433,6 +433,15 @@ hs-adjust-block-beginning
 
 See `hs-c-like-adjust-block-beginning' for an example of using this.")
 
+(defvar-local hs-find-block-beginning-func #'hs-find-block-beginning
+  "Function used to do `hs-find-block-beginning'.")
+
+(defvar-local hs-find-next-block-func #'hs-find-next-block
+  "Function used to do `hs-find-next-block'.")
+
+(defvar-local hs-looking-at-block-start-p-func #'hs-looking-at-block-start-p
+  "Function used to do hs-looking-at-block-start-p.")
+
 (defvar hs-headline nil
   "Text of the line where a hidden block begins, set during isearch.
 You can display this in the mode line by adding the symbol `hs-headline'
@@ -565,7 +574,7 @@ hs-hide-block-at-point
 and then further adjusted to be at the end of the line."
   (if comment-reg
       (hs-hide-comment-region (car comment-reg) (cadr comment-reg) end)
-    (when (hs-looking-at-block-start-p)
+    (when (funcall hs-looking-at-block-start-p-func)
       (let ((mdata (match-data t))
             (header-end (match-end 0))
             p q ov)
@@ -672,7 +681,14 @@ hs-grok-mode-type
                                                      0 (1- (match-end 0)))
                                         c-start-regexp)))
               hs-forward-sexp-func (or (nth 4 lookup) #'forward-sexp)
-              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)))
+              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)
+              hs-find-block-beginning-func (or (nth 6 lookup)
+                                               #'hs-find-block-beginning)
+              hs-find-next-block-func (or (nth 7 lookup)
+                                          #'hs-find-next-block)
+              hs-looking-at-block-start-p-func
+              (or (nth 8 lookup)
+                  #'hs-looking-at-block-start-p)))
     (setq hs-minor-mode nil)
     (error "%s Mode doesn't support Hideshow Minor Mode"
            (format-mode-line mode-name))))
@@ -683,7 +699,7 @@ hs-find-block-beginning
   (let ((done nil)
         (here (point)))
     ;; look if current line is block start
-    (if (hs-looking-at-block-start-p)
+    (if (funcall hs-looking-at-block-start-p-func)
         (point)
       ;; look backward for the start of a block that contains the cursor
       (while (and (re-search-backward hs-block-start-regexp nil t)
@@ -698,19 +714,25 @@ hs-find-block-beginning
         (goto-char here)
         nil))))
 
+(defun hs-find-next-block (regexp maxp skip-comments)
+  "Reposition point at next block-start.
+Skip comments if SKIP-COMMENTS is not nil, and search for REGEXP
+in region (point MAXP)."
+  (when skip-comments
+    (forward-comment (point-max)))
+  (and (< (point) maxp)
+       (re-search-forward regexp maxp t)))
+
 (defun hs-hide-level-recursive (arg minp maxp)
   "Recursively hide blocks ARG levels below point in region (MINP MAXP)."
-  (when (hs-find-block-beginning)
+  (when (funcall hs-find-block-beginning-func)
     (setq minp (1+ (point)))
     (funcall hs-forward-sexp-func 1)
     (setq maxp (1- (point))))
   (unless hs-allow-nesting
     (hs-discard-overlays minp maxp))
   (goto-char minp)
-  (while (progn
-           (forward-comment (buffer-size))
-           (and (< (point) maxp)
-                (re-search-forward hs-block-start-regexp maxp t)))
+  (while (funcall hs-find-next-block-func hs-block-start-regexp maxp t)
     (when (save-match-data
 	    (not (nth 8 (syntax-ppss)))) ; not inside comments or strings
       (if (> arg 1)
@@ -747,8 +769,8 @@ hs-already-hidden-p
           (goto-char (nth 0 c-reg))
         (end-of-line)
         (when (and (not c-reg)
-                   (hs-find-block-beginning)
-		   (hs-looking-at-block-start-p))
+                   (funcall hs-find-block-beginning-func)
+		   (funcall hs-looking-at-block-start-p-func))
           ;; point is inside a block
           (goto-char (match-end 0)))))
     (end-of-line)
@@ -790,10 +812,8 @@ hs-hide-all
                                    hs-c-start-regexp
                                    "\\)")
                          ""))))
-       (while (progn
-                (unless hs-hide-comments-when-hiding-all
-                  (forward-comment (point-max)))
-                (re-search-forward re (point-max) t))
+       (while (funcall hs-find-next-block-func re (point-max)
+                       (not hs-hide-comments-when-hiding-all))
          (if (match-beginning 1)
              ;; We have found a block beginning.
              (progn
@@ -838,8 +858,8 @@ hs-hide-block
                       (<= (count-lines (car c-reg) (nth 1 c-reg)) 1)))
        (message "(not enough comment lines to hide)"))
       ((or c-reg
-	   (hs-looking-at-block-start-p)
-           (hs-find-block-beginning))
+	   (funcall hs-looking-at-block-start-p-func)
+           (funcall hs-find-block-beginning-func))
        (hs-hide-block-at-point end c-reg)
        (run-hooks 'hs-hide-hook))))))
 
@@ -868,9 +888,9 @@ hs-show-block
              (when (car c-reg)
                (setq p (car c-reg)
                      q (cadr c-reg))))
-            ((and (hs-find-block-beginning)
+            ((and (funcall hs-find-block-beginning-func)
                   ;; ugh, fresh match-data
-                  (hs-looking-at-block-start-p))
+                  (funcall hs-looking-at-block-start-p-func))
              (setq p (point)
                    q (progn (hs-forward-sexp (match-data t) 1) (point)))))
       (when (and p q)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 88b19c88cd..3bcadf1254 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1496,6 +1496,10 @@ python-nav-beginning-of-defun-regexp
 The name of the defun should be grouped so it can be retrieved
 via `match-string'.")
 
+(defvar python-nav-beginning-of-block-regexp
+  (python-rx line-start (* space) block-start)
+  "Regexp matching block start.")
+
 (defun python-nav--beginning-of-defun (&optional arg)
   "Internal implementation of `python-nav-beginning-of-defun'.
 With positive ARG search backwards, else search forwards."
@@ -4877,9 +4881,29 @@ python-describe-at-point
 (defun python-hideshow-forward-sexp-function (_arg)
   "Python specific `forward-sexp' function for `hs-minor-mode'.
 Argument ARG is ignored."
-  (python-nav-end-of-defun)
-  (unless (python-info-current-line-empty-p)
-    (backward-char)))
+  (python-nav-end-of-block))
+
+(defun python-hideshow-find-block-beginning ()
+  "Python specific `hs-find-block-beginning' furnction for `hs-minor-mode'."
+  (let ((starting-pos (point))
+        (block-beginning))
+    (save-excursion
+      (when (and (python-nav-beginning-of-block)
+                 (<= starting-pos (save-excursion (python-nav-end-of-block))))
+        (setq block-beginning (point))))
+    (when block-beginning
+      (goto-char block-beginning))))
+
+(defun python-hideshow-find-next-block (regexp maxp _skip-comments)
+  "Python specific `hs-find-next-block' furnction for `hs-minor-mode'.
+Call `python-nav-forward-block' to find next block and check if point <= MAXP.
+REGEXP is passed to `looking-at' to set `match-data'."
+  (let ((next-block (save-excursion (python-nav-forward-block))))
+    (when (and next-block (<= next-block maxp))
+      (goto-char next-block)
+      (save-excursion
+        (beginning-of-line)
+        (looking-at regexp)))))
 
 \f
 ;;; Imenu
@@ -5376,6 +5400,18 @@ python-info-looking-at-beginning-of-defun
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
+(defun python-info-looking-at-beginning-of-block ()
+  "Check if point is at beginning of block."
+  (let* ((line-beg-pos (line-beginning-position))
+         (line-content-start (+ line-beg-pos (current-indentation))))
+    (and (= (save-excursion
+              (python-nav-beginning-of-block))
+            line-content-start)
+         (<= (point) line-content-start)
+         (save-excursion
+           (beginning-of-line)
+           (looking-at python-nav-beginning-of-block-regexp)))))
+
 (defun python-info-current-line-comment-p ()
   "Return non-nil if current line is a comment line."
   (char-equal
@@ -5825,14 +5861,17 @@ python-mode
 
   (add-to-list
    'hs-special-modes-alist
-   '(python-mode
-     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
+   `(python-mode
+     ,python-nav-beginning-of-block-regexp
      ;; Use the empty string as end regexp so it doesn't default to
      ;; "\\s)".  This way parens at end of defun are properly hidden.
      ""
      "#"
      python-hideshow-forward-sexp-function
-     nil))
+     nil
+     python-hideshow-find-block-beginning
+     python-hideshow-find-next-block
+     python-info-looking-at-beginning-of-block))
 
   (setq-local outline-regexp (python-rx (* space) block-start))
   (setq-local outline-heading-end-regexp ":[^\n]*\n")
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index eb57122690..e5348fdde4 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5956,8 +5956,11 @@ python-hideshow-hide-levels-1
 class SomeClass:
 
     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))
 
 (ert-deftest python-hideshow-hide-levels-2 ()

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-07 14:37             ` kobarity
@ 2022-08-08 15:16               ` Augusto Stoffel
  2022-08-13 12:36                 ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Augusto Stoffel @ 2022-08-08 15:16 UTC (permalink / raw)
  To: kobarity; +Cc: Lars Ingebrigtsen, Dima Kogan, 56635

This is a marginally related remark, but I noticed that python-mode sets

    (setq-local outline-heading-end-regexp ":[^\n]*\n")

and this doesn't work well with the new-ish type annotation syntax.

Wouldn't it be better to simply remove this setting, reverting back to
the default "\n"?  Then folding by outline-minor-mode would behave like
hideshow currently does when it comes to something like

    def f(
        x: int
    ) -> int:
        pass

On Sun,  7 Aug 2022 at 23:37, kobarity <kobarity@gmail.com> wrote:

> Hi,
>
> Dima Kogan <dima@secretsauce.net> wrote:
>> I do agree that the best thing to do is to use functions instead of pure
>> regexes here. Can we hook that into hs-special-modes-alist instead of
>> doing something specific in python-mode? I'm imagining extending the
>> meaning of the fields in hs-special-modes-alist, where we'd do something
>> different, depending on if some element is a string or not. Maybe that
>> would make it easier to extend the hideshow support for other languages?
>
> If we are to extend hs-special-modes-alist, I think it's better to add
> functions to find/check blocks. Maybe this is the third option
> mentioned in the first mail.  For that, I added three functions to
> hs-special-modes-alist:
>
> - hs-find-block-beginning-func
> - hs-find-next-block-func
> - hs-looking-at-block-start-p-func
>
> Attached is a prototype patch to achieve this.
>
> Regards,





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-08 15:16               ` Augusto Stoffel
@ 2022-08-13 12:36                 ` kobarity
  2022-08-15 15:00                   ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-08-13 12:36 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: Lars Ingebrigtsen, Dima Kogan, 56635

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

Augusto Stoffel <arstoffel@gmail.com> wrote:
>
> This is a marginally related remark, but I noticed that python-mode sets
>
>     (setq-local outline-heading-end-regexp ":[^\n]*\n")
>
> and this doesn't work well with the new-ish type annotation syntax.
>
> Wouldn't it be better to simply remove this setting, reverting back to
> the default "\n"?  Then folding by outline-minor-mode would behave like
> hideshow currently does when it comes to something like
>
>     def f(
>         x: int
>     ) -> int:
>         pass

I agree with removing the `outline-heading-end-regexp' setting.  I thought
about adding a custom variable since some people might prefer the
current setting, but I reconsidered that setting it as a file local
variable would be sufficient as described in docstring:

#+begin_quote
The recommended way to set this is with a ‘Local Variables:’ list
in the file it applies to.
#+end_quote

Is there any comments on extending `hs-special-modes-alist'?  Attached
are revised patches.  I would like to know if it is worth extending
`hs-special-modes-alist'.

Best Regards,

[-- Attachment #2: fix-56635-hideshow.patch --]
[-- Type: application/octet-stream, Size: 15515 bytes --]

commit b163567610f309f1fd264bcb46bb81c6f8013cde
Author: kobarity <kobarity@gmail.com>
Date:   Sat Aug 13 16:22:40 2022 +0900

    Extend `hs-special-modes-alist' for languages such as Python
    
    * lisp/progmodes/hideshow.el (hs-special-modes-alist): Add
    elements FIND-BLOCK-BEGINNING-FUNC, FIND-NEXT-BLOCK-FUNC, and
    LOOKING-AT-BLOCK-START-P-FUNC.
    (hs-find-block-beginning-func): New variable to hold
    FIND-BLOCK-BEGINNING-FUNC.
    (hs-find-next-block-func): New variable to hold
    FIND-NEXT-BLOCK-FUNC.
    (hs-looking-at-block-start-p-func): New variable to hold
    LOOKING-AT-BLOCK-START-P-FUNC.
    (hs-grok-mode-type): Set new variables from
    `hs-special-modes-alist'.
    (hs-find-next-block): New function.
    (Misc.): Update callers of the above functions.
    
    * test/lisp/progmodes/hideshow-tests.el: New test file.

diff --git a/lisp/progmodes/hideshow.el b/lisp/progmodes/hideshow.el
index f574ec84fb..c0796fc2ee 100644
--- a/lisp/progmodes/hideshow.el
+++ b/lisp/progmodes/hideshow.el
@@ -267,7 +267,9 @@ hs-special-modes-alist
     ))
   "Alist for initializing the hideshow variables for different modes.
 Each element has the form
-  (MODE START END COMMENT-START FORWARD-SEXP-FUNC ADJUST-BEG-FUNC).
+  (MODE START END COMMENT-START FORWARD-SEXP-FUNC ADJUST-BEG-FUNC
+   FIND-BLOCK-BEGINNING-FUNC FIND-NEXT-BLOCK-FUNC
+   LOOKING-AT-BLOCK-START-P-FUNC).
 
 If non-nil, hideshow will use these values as regexps to define blocks
 and comments, respectively for major mode MODE.
@@ -288,6 +290,15 @@ hs-special-modes-alist
 See the documentation for `hs-adjust-block-beginning' to see what is the
 use of ADJUST-BEG-FUNC.
 
+See the documentation for `hs-find-block-beginning-func' to see
+what is the use of FIND-BLOCK-BEGINNING-FUNC.
+
+See the documentation for `hs-find-next-block-func' to see what
+is the use of FIND-NEXT-BLOCK-FUNC.
+
+See the documentation for `hs-looking-at-block-start-p-func' to
+see what is the use of LOOKING-AT-BLOCK-START-P-FUNC.
+
 If any of the elements is left nil or omitted, hideshow tries to guess
 appropriate values.  The regexps should not contain leading or trailing
 whitespace.  Case does not matter.")
@@ -433,6 +444,39 @@ hs-adjust-block-beginning
 
 See `hs-c-like-adjust-block-beginning' for an example of using this.")
 
+(defvar-local hs-find-block-beginning-func #'hs-find-block-beginning
+  "Function used to do `hs-find-block-beginning'.
+It should reposition point at the beginning of the current block
+and return point, or nil if original point was not in a block.
+
+Specifying this function is necessary for languages such as
+Python, where regexp search and `syntax-ppss' check is not enough
+to find the beginning of the current block.")
+
+(defvar-local hs-find-next-block-func #'hs-find-next-block
+  "Function used to do `hs-find-next-block'.
+It should reposition point at next block start.
+
+It is called with three arguments REGEXP, MAXP, and COMMENTS.
+REGEXP is a regexp representing block start.  When block start is
+found, `match-data' should be set using REGEXP.  MAXP is a buffer
+position that bounds the search.  When COMMENTS is nil, comments
+should be skipped.  When COMMENTS is not nil, REGEXP matches not
+only beginning of a block but also beginning of a comment.  In
+this case, the function should find nearest block or comment.
+
+Specifying this function is necessary for languages such as
+Python, where regexp search is not enough to find the beginning
+of the next block.")
+
+(defvar-local hs-looking-at-block-start-p-func #'hs-looking-at-block-start-p
+  "Function used to do `hs-looking-at-block-start-p'.
+It should return non-nil if the point is at the block start.
+
+Specifying this function is necessary for languages such as
+Python, where `looking-at' and `syntax-ppss' check is not enough
+to check if the point is at the block start.")
+
 (defvar hs-headline nil
   "Text of the line where a hidden block begins, set during isearch.
 You can display this in the mode line by adding the symbol `hs-headline'
@@ -565,7 +609,7 @@ hs-hide-block-at-point
 and then further adjusted to be at the end of the line."
   (if comment-reg
       (hs-hide-comment-region (car comment-reg) (cadr comment-reg) end)
-    (when (hs-looking-at-block-start-p)
+    (when (funcall hs-looking-at-block-start-p-func)
       (let ((mdata (match-data t))
             (header-end (match-end 0))
             p q ov)
@@ -672,7 +716,14 @@ hs-grok-mode-type
                                                      0 (1- (match-end 0)))
                                         c-start-regexp)))
               hs-forward-sexp-func (or (nth 4 lookup) #'forward-sexp)
-              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)))
+              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)
+              hs-find-block-beginning-func (or (nth 6 lookup)
+                                               #'hs-find-block-beginning)
+              hs-find-next-block-func (or (nth 7 lookup)
+                                          #'hs-find-next-block)
+              hs-looking-at-block-start-p-func
+              (or (nth 8 lookup)
+                  #'hs-looking-at-block-start-p)))
     (setq hs-minor-mode nil)
     (error "%s Mode doesn't support Hideshow Minor Mode"
            (format-mode-line mode-name))))
@@ -683,7 +734,7 @@ hs-find-block-beginning
   (let ((done nil)
         (here (point)))
     ;; look if current line is block start
-    (if (hs-looking-at-block-start-p)
+    (if (funcall hs-looking-at-block-start-p-func)
         (point)
       ;; look backward for the start of a block that contains the cursor
       (while (and (re-search-backward hs-block-start-regexp nil t)
@@ -698,19 +749,25 @@ hs-find-block-beginning
         (goto-char here)
         nil))))
 
+(defun hs-find-next-block (regexp maxp comments)
+  "Reposition point at next block-start.
+Skip comments if COMMENTS is nil, and search for REGEXP in
+region (point MAXP)."
+  (when (not comments)
+    (forward-comment (point-max)))
+  (and (< (point) maxp)
+       (re-search-forward regexp maxp t)))
+
 (defun hs-hide-level-recursive (arg minp maxp)
   "Recursively hide blocks ARG levels below point in region (MINP MAXP)."
-  (when (hs-find-block-beginning)
+  (when (funcall hs-find-block-beginning-func)
     (setq minp (1+ (point)))
     (funcall hs-forward-sexp-func 1)
     (setq maxp (1- (point))))
   (unless hs-allow-nesting
     (hs-discard-overlays minp maxp))
   (goto-char minp)
-  (while (progn
-           (forward-comment (buffer-size))
-           (and (< (point) maxp)
-                (re-search-forward hs-block-start-regexp maxp t)))
+  (while (funcall hs-find-next-block-func hs-block-start-regexp maxp nil)
     (when (save-match-data
 	    (not (nth 8 (syntax-ppss)))) ; not inside comments or strings
       (if (> arg 1)
@@ -747,8 +804,8 @@ hs-already-hidden-p
           (goto-char (nth 0 c-reg))
         (end-of-line)
         (when (and (not c-reg)
-                   (hs-find-block-beginning)
-		   (hs-looking-at-block-start-p))
+                   (funcall hs-find-block-beginning-func)
+		   (funcall hs-looking-at-block-start-p-func))
           ;; point is inside a block
           (goto-char (match-end 0)))))
     (end-of-line)
@@ -790,10 +847,8 @@ hs-hide-all
                                    hs-c-start-regexp
                                    "\\)")
                          ""))))
-       (while (progn
-                (unless hs-hide-comments-when-hiding-all
-                  (forward-comment (point-max)))
-                (re-search-forward re (point-max) t))
+       (while (funcall hs-find-next-block-func re (point-max)
+                       hs-hide-comments-when-hiding-all)
          (if (match-beginning 1)
              ;; We have found a block beginning.
              (progn
@@ -838,8 +893,8 @@ hs-hide-block
                       (<= (count-lines (car c-reg) (nth 1 c-reg)) 1)))
        (message "(not enough comment lines to hide)"))
       ((or c-reg
-	   (hs-looking-at-block-start-p)
-           (hs-find-block-beginning))
+	   (funcall hs-looking-at-block-start-p-func)
+           (funcall hs-find-block-beginning-func))
        (hs-hide-block-at-point end c-reg)
        (run-hooks 'hs-hide-hook))))))
 
@@ -868,9 +923,9 @@ hs-show-block
              (when (car c-reg)
                (setq p (car c-reg)
                      q (cadr c-reg))))
-            ((and (hs-find-block-beginning)
+            ((and (funcall hs-find-block-beginning-func)
                   ;; ugh, fresh match-data
-                  (hs-looking-at-block-start-p))
+                  (funcall hs-looking-at-block-start-p-func))
              (setq p (point)
                    q (progn (hs-forward-sexp (match-data t) 1) (point)))))
       (when (and p q)
diff --git a/test/lisp/progmodes/hideshow-tests.el b/test/lisp/progmodes/hideshow-tests.el
new file mode 100644
index 0000000000..ee2a0c7c4c
--- /dev/null
+++ b/test/lisp/progmodes/hideshow-tests.el
@@ -0,0 +1,268 @@
+;;; hideshow-tests.el --- Test suite for hideshow.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'hideshow)
+
+;; Dependencies for testing:
+(require 'cc-mode)
+
+
+(defmacro hideshow-tests-with-temp-buffer (mode contents &rest body)
+  "Create a `hs-minor-mode' enabled MODE temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (,mode)
+     (hs-minor-mode 1)
+     (insert ,contents)
+     (goto-char (point-min))
+     ,@body))
+
+(defun hideshow-tests-look-at (string &optional num restore-point)
+  "Move point at beginning of STRING in the current buffer.
+Optional argument NUM defaults to 1 and is an integer indicating
+how many occurrences must be found, when positive the search is
+done forwards, otherwise backwards.  When RESTORE-POINT is
+non-nil the point is not moved but the position found is still
+returned.  When searching forward and point is already looking at
+STRING, it is skipped so the next STRING occurrence is selected."
+  (let* ((num (or num 1))
+         (starting-point (point))
+         (string (regexp-quote string))
+         (search-fn (if (> num 0) #'re-search-forward #'re-search-backward))
+         (deinc-fn (if (> num 0) #'1- #'1+))
+         (found-point))
+    (prog2
+        (catch 'exit
+          (while (not (= num 0))
+            (when (and (> num 0)
+                       (looking-at string))
+              ;; Moving forward and already looking at STRING, skip it.
+              (forward-char (length (match-string-no-properties 0))))
+            (and (not (funcall search-fn string nil t))
+                 (throw 'exit t))
+            (when (> num 0)
+              ;; `re-search-forward' leaves point at the end of the
+              ;; occurrence, move back so point is at the beginning
+              ;; instead.
+              (forward-char (- (length (match-string-no-properties 0)))))
+            (setq
+             num (funcall deinc-fn num)
+             found-point (point))))
+        found-point
+      (and restore-point (goto-char starting-point)))))
+
+(defun hideshow-tests-visible-string (&optional min max)
+  "Return the buffer string excluding invisible overlays.
+Argument MIN and MAX delimit the region to be returned and
+default to `point-min' and `point-max' respectively."
+  (let* ((min (or min (point-min)))
+         (max (or max (point-max)))
+         (buffer-contents (buffer-substring-no-properties min max))
+         (overlays
+          (sort (overlays-in min max)
+                (lambda (a b)
+                  (let ((overlay-end-a (overlay-end a))
+                        (overlay-end-b (overlay-end b)))
+                    (> overlay-end-a overlay-end-b))))))
+    (with-temp-buffer
+      (insert buffer-contents)
+      (dolist (overlay overlays)
+        (if (overlay-get overlay 'invisible)
+            (delete-region (overlay-start overlay)
+                           (overlay-end overlay))))
+      (buffer-substring-no-properties (point-min) (point-max)))))
+
+(ert-deftest hideshow-hide-block-1 ()
+  "Should hide current block."
+  (let ((contents "
+int
+main()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (hideshow-tests-look-at "printf")
+     (hs-hide-block)
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+int
+main()
+{}
+"))
+     (hs-show-block)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-all-1 ()
+  "Should hide all blocks and comments."
+  (let ((contents "
+/*
+   Comments
+*/
+
+int
+main()
+{
+  sub();
+}
+
+void
+sub()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (hs-hide-all)
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+/*
+
+int
+main()
+{}
+
+void
+sub()
+{}
+"))
+     (hs-show-all)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-all-2 ()
+  "Should not hide comments when `hs-hide-comments-when-hiding-all' is nil."
+  (let ((contents "
+/*
+   Comments
+*/
+
+int
+main()
+{
+  sub();
+}
+
+void
+sub()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (let ((hs-hide-comments-when-hiding-all nil))
+       (hs-hide-all))
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+/*
+   Comments
+*/
+
+int
+main()
+{}
+
+void
+sub()
+{}
+"))
+     (hs-show-all)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-level-1 ()
+  "Should hide 1st level blocks."
+  (hideshow-tests-with-temp-buffer
+   c-mode
+   "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {
+    printf(\"Hello\\n\");
+  }
+}
+"
+   (hs-hide-level 1)
+   (should (string=
+            (hideshow-tests-visible-string)
+            "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{}
+"))))
+
+(ert-deftest hideshow-hide-level-2 ()
+  "Should hide 2nd level blocks."
+  (hideshow-tests-with-temp-buffer
+   c-mode
+   "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {
+    printf(\"Hello\\n\");
+  }
+}
+"
+   (hs-hide-level 2)
+   (should (string=
+            (hideshow-tests-visible-string)
+            "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {}
+}
+"))))
+
+(provide 'hideshow-tests)
+
+;;; hideshow-tests.el ends here

[-- Attachment #3: fix-56635-python.patch --]
[-- Type: application/octet-stream, Size: 9187 bytes --]

commit eb3e9d1870eecb2f66d47524a825cd3a28c9bdde
Author: kobarity <kobarity@gmail.com>
Date:   Sat Aug 13 20:33:46 2022 +0900

    Add Python blocks support for hideshow
    
    * lisp/progmodes/python.el (python-nav-beginning-of-block-regexp):
    New variable.
    (python-hideshow-forward-sexp-function): Change to call
    `python-nav-end-of-block'.
    (python-hideshow-find-next-block): New function to be used as
    FIND-NEXT-BLOCK-FUNC in `hs-special-modes-alist'.
    (python-info-looking-at-beginning-of-block): New function to be
    used as LOOKING-AT-BLOCK-START-P-FUNC in `hs-special-modes-alist'.
    (python-mode): Change settings of `hs-special-modes-alist'.
    
    * test/lisp/progmodes/python-tests.el
    (python-hideshow-hide-levels-1): Fix to keep empty lines.
    (python-info-looking-at-beginning-of-block-1)
    (python-hideshow-hide-levels-3, python-hideshow-hide-levels-4)
    (python-hideshow-hide-all-1, python-hideshow-hide-all-2)
    (python-hideshow-hide-all-3, python-hideshow-hide-block-1): New
    tests.

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 96f9d14832..938ca43e86 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1504,6 +1504,10 @@ python-nav-beginning-of-defun-regexp
 The name of the defun should be grouped so it can be retrieved
 via `match-string'.")
 
+(defvar python-nav-beginning-of-block-regexp
+  (python-rx line-start (* space) block-start)
+  "Regexp matching block start.")
+
 (defun python-nav--beginning-of-defun (&optional arg)
   "Internal implementation of `python-nav-beginning-of-defun'.
 With positive ARG search backwards, else search forwards."
@@ -4885,9 +4889,32 @@ python-describe-at-point
 (defun python-hideshow-forward-sexp-function (_arg)
   "Python specific `forward-sexp' function for `hs-minor-mode'.
 Argument ARG is ignored."
-  (python-nav-end-of-defun)
-  (unless (python-info-current-line-empty-p)
-    (backward-char)))
+  (python-nav-end-of-block))
+
+(defun python-hideshow-find-next-block (regexp maxp comments)
+  "Python specific `hs-find-next-block' function for `hs-minor-mode'.
+Call `python-nav-forward-block' to find next block and check if
+block-start ends within MAXP.  If COMMENTS is not nil, comments
+are also searched.  REGEXP is passed to `looking-at' to set
+`match-data'."
+  (let* ((next-block (or (save-excursion
+                           (and (python-nav-forward-block)
+                                (re-search-forward
+                                 (python-rx block-start) maxp t)))
+                         (1+ maxp)))
+         (next-comment
+          (or (when comments
+                (save-excursion
+                  (cl-loop while (re-search-forward "#" maxp t)
+                           if (python-syntax-context 'comment)
+                           return (point))))
+              (1+ maxp)))
+         (next-block-or-comment (min next-block next-comment)))
+    (when (<= next-block-or-comment maxp)
+      (goto-char next-block-or-comment)
+      (save-excursion
+        (beginning-of-line)
+        (looking-at regexp)))))
 
 \f
 ;;; Imenu
@@ -5384,6 +5411,19 @@ python-info-looking-at-beginning-of-defun
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
+(defun python-info-looking-at-beginning-of-block ()
+  "Check if point is at the beginning of block."
+  (let* ((line-beg-pos (line-beginning-position))
+         (line-content-start (+ line-beg-pos (current-indentation)))
+         (block-beg-pos (save-excursion
+                          (python-nav-beginning-of-block))))
+    (and block-beg-pos
+         (= block-beg-pos line-content-start)
+         (<= (point) line-content-start)
+         (save-excursion
+           (beginning-of-line)
+           (looking-at python-nav-beginning-of-block-regexp)))))
+
 (defun python-info-current-line-comment-p ()
   "Return non-nil if current line is a comment line."
   (char-equal
@@ -5833,14 +5873,17 @@ python-mode
 
   (add-to-list
    'hs-special-modes-alist
-   '(python-mode
-     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
+   `(python-mode
+     ,python-nav-beginning-of-block-regexp
      ;; Use the empty string as end regexp so it doesn't default to
      ;; "\\s)".  This way parens at end of defun are properly hidden.
      ""
      "#"
      python-hideshow-forward-sexp-function
-     nil))
+     nil
+     python-nav-beginning-of-block
+     python-hideshow-find-next-block
+     python-info-looking-at-beginning-of-block))
 
   (setq-local outline-regexp (python-rx (* space) block-start))
   (setq-local outline-heading-end-regexp ":[^\n]*\n")
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index d303050fad..2fa91dc79b 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5582,6 +5582,39 @@ python-info-looking-at-beginning-of-defun-2
    (should (not (python-info-looking-at-beginning-of-defun)))
    (should (not (python-info-looking-at-beginning-of-defun nil t)))))
 
+(ert-deftest python-info-looking-at-beginning-of-block-1 ()
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if True:
+        pass
+    l = [x * 2
+         for x in range(5)
+         if x < 3]
+# if False:
+\"\"\"
+if 0:
+\"\"\"
+"
+   (python-tests-look-at "def f():")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if True:")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (beginning-of-line)
+   (should (python-info-looking-at-beginning-of-block))
+   (python-tests-look-at "for x")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if x < 3")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if False:")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if 0:")
+   (should (not (python-info-looking-at-beginning-of-block)))))
+
 (ert-deftest python-info-current-line-comment-p-1 ()
   (python-tests-with-temp-buffer
    "
@@ -6035,8 +6068,11 @@ python-hideshow-hide-levels-1
 class SomeClass:
 
     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))
 
 (ert-deftest python-hideshow-hide-levels-2 ()
@@ -6082,6 +6118,156 @@ python-hideshow-hide-levels-2
 "))))
       (or enabled (hs-minor-mode -1)))))
 
+(ert-deftest python-hideshow-hide-levels-3 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 1)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+"))))
+
+(ert-deftest python-hideshow-hide-levels-4 ()
+  "Should hide 2nd level block."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 2)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+    if 0:
+"))))
+
+(ert-deftest python-hideshow-hide-all-1 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "
+if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+if 0:
+
+def f():
+"))))
+
+(ert-deftest python-hideshow-hide-all-2 ()
+  "Should hide comments."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-all-3 ()
+  "Should not hide comments when `hs-hide-comments-when-hiding-all' is nil."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (let ((hs-hide-comments-when-hiding-all nil))
+     (hs-hide-all))
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-block-1 ()
+  "Should hide current block."
+  (python-tests-with-temp-buffer
+   "
+if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (python-tests-look-at "ddd")
+   (forward-line)
+   (hs-hide-block)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+if 0:
+
+def f():
+    pass
+"))))
+
 
 (ert-deftest python-tests--python-nav-end-of-statement--infloop ()
   "Checks that `python-nav-end-of-statement' doesn't infloop in a

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-13 12:36                 ` kobarity
@ 2022-08-15 15:00                   ` kobarity
  2022-08-15 17:16                     ` Dima Kogan
  2022-08-17 10:45                     ` Lars Ingebrigtsen
  0 siblings, 2 replies; 21+ messages in thread
From: kobarity @ 2022-08-15 15:00 UTC (permalink / raw)
  To: Augusto Stoffel, Dima Kogan, Lars Ingebrigtsen, 56635

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

I wrote:
> Is there any comments on extending `hs-special-modes-alist'?  Attached
> are revised patches.  I would like to know if it is worth extending
> `hs-special-modes-alist'.

I fixed some issues in the Python patch.  Please note that the fix for
#57223 (merged recently) is also needed for all ERTs to pass.

Best Regards,

[-- Attachment #2: fix-56635-python.patch --]
[-- Type: application/octet-stream, Size: 9526 bytes --]

commit 177a3a9f9a5b39c1c5df6dc641d527d8803bc29e
Author: kobarity <kobarity@gmail.com>
Date:   Mon Aug 15 23:49:34 2022 +0900

    Add Python blocks support for hideshow
    
    * lisp/progmodes/python.el (python-nav-beginning-of-block-regexp):
    New variable.
    (python-hideshow-forward-sexp-function): Change to call
    `python-nav-end-of-block'.
    (python-hideshow-find-next-block): New function to be used as
    FIND-NEXT-BLOCK-FUNC in `hs-special-modes-alist'.
    (python-info-looking-at-beginning-of-block): New function to be
    used as LOOKING-AT-BLOCK-START-P-FUNC in `hs-special-modes-alist'.
    (python-mode): Change settings of `hs-special-modes-alist'.
    
    * test/lisp/progmodes/python-tests.el
    (python-hideshow-hide-levels-1): Fix to keep empty lines.
    (python-info-looking-at-beginning-of-block-1)
    (python-hideshow-hide-levels-3, python-hideshow-hide-levels-4)
    (python-hideshow-hide-all-1, python-hideshow-hide-all-2)
    (python-hideshow-hide-all-3, python-hideshow-hide-block-1): New
    tests.

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 44df3186b2..295636c32e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1504,6 +1504,10 @@ python-nav-beginning-of-defun-regexp
 The name of the defun should be grouped so it can be retrieved
 via `match-string'.")
 
+(defvar python-nav-beginning-of-block-regexp
+  (python-rx line-start (* space) block-start)
+  "Regexp matching block start.")
+
 (defun python-nav--beginning-of-defun (&optional arg)
   "Internal implementation of `python-nav-beginning-of-defun'.
 With positive ARG search backwards, else search forwards."
@@ -4887,9 +4891,37 @@ python-describe-at-point
 (defun python-hideshow-forward-sexp-function (_arg)
   "Python specific `forward-sexp' function for `hs-minor-mode'.
 Argument ARG is ignored."
-  (python-nav-end-of-defun)
-  (unless (python-info-current-line-empty-p)
-    (backward-char)))
+  (python-nav-end-of-block))
+
+(defun python-hideshow-find-next-block (regexp maxp comments)
+  "Python specific `hs-find-next-block' function for `hs-minor-mode'.
+Call `python-nav-forward-block' to find next block and check if
+block-start ends within MAXP.  If COMMENTS is not nil, comments
+are also searched.  REGEXP is passed to `looking-at' to set
+`match-data'."
+  (let* ((next-block (save-excursion
+                       (or (and
+                            (python-info-looking-at-beginning-of-block)
+                            (re-search-forward
+                             (python-rx block-start) maxp t))
+                           (and (python-nav-forward-block)
+                                (< (point) maxp)
+                                (re-search-forward
+                                 (python-rx block-start) maxp t))
+                           (1+ maxp))))
+         (next-comment
+          (or (when comments
+                (save-excursion
+                  (cl-loop while (re-search-forward "#" maxp t)
+                           if (python-syntax-context 'comment)
+                           return (point))))
+              (1+ maxp)))
+         (next-block-or-comment (min next-block next-comment)))
+    (when (<= next-block-or-comment maxp)
+      (goto-char next-block-or-comment)
+      (save-excursion
+        (beginning-of-line)
+        (looking-at regexp)))))
 
 \f
 ;;; Imenu
@@ -5386,6 +5418,19 @@ python-info-looking-at-beginning-of-defun
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
+(defun python-info-looking-at-beginning-of-block ()
+  "Check if point is at the beginning of block."
+  (let* ((line-beg-pos (line-beginning-position))
+         (line-content-start (+ line-beg-pos (current-indentation)))
+         (block-beg-pos (save-excursion
+                          (python-nav-beginning-of-block))))
+    (and block-beg-pos
+         (= block-beg-pos line-content-start)
+         (<= (point) line-content-start)
+         (save-excursion
+           (beginning-of-line)
+           (looking-at python-nav-beginning-of-block-regexp)))))
+
 (defun python-info-current-line-comment-p ()
   "Return non-nil if current line is a comment line."
   (char-equal
@@ -5835,14 +5880,17 @@ python-mode
 
   (add-to-list
    'hs-special-modes-alist
-   '(python-mode
-     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
+   `(python-mode
+     ,python-nav-beginning-of-block-regexp
      ;; Use the empty string as end regexp so it doesn't default to
      ;; "\\s)".  This way parens at end of defun are properly hidden.
      ""
      "#"
      python-hideshow-forward-sexp-function
-     nil))
+     nil
+     python-nav-beginning-of-block
+     python-hideshow-find-next-block
+     python-info-looking-at-beginning-of-block))
 
   (setq-local outline-regexp (python-rx (* space) block-start))
   (setq-local outline-heading-end-regexp ":[^\n]*\n")
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 9e8fa7f552..608ce548e7 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5598,6 +5598,39 @@ python-info-looking-at-beginning-of-defun-2
    (should (not (python-info-looking-at-beginning-of-defun)))
    (should (not (python-info-looking-at-beginning-of-defun nil t)))))
 
+(ert-deftest python-info-looking-at-beginning-of-block-1 ()
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if True:
+        pass
+    l = [x * 2
+         for x in range(5)
+         if x < 3]
+# if False:
+\"\"\"
+if 0:
+\"\"\"
+"
+   (python-tests-look-at "def f():")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if True:")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (beginning-of-line)
+   (should (python-info-looking-at-beginning-of-block))
+   (python-tests-look-at "for x")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if x < 3")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if False:")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if 0:")
+   (should (not (python-info-looking-at-beginning-of-block)))))
+
 (ert-deftest python-info-current-line-comment-p-1 ()
   (python-tests-with-temp-buffer
    "
@@ -6051,8 +6084,11 @@ python-hideshow-hide-levels-1
 class SomeClass:
 
     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))
 
 (ert-deftest python-hideshow-hide-levels-2 ()
@@ -6098,6 +6134,165 @@ python-hideshow-hide-levels-2
 "))))
       (or enabled (hs-minor-mode -1)))))
 
+(ert-deftest python-hideshow-hide-levels-3 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+
+def g():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 1)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+
+def g():
+"))))
+
+(ert-deftest python-hideshow-hide-levels-4 ()
+  "Should hide 2nd level block."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+
+def g():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 2)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+    if 0:
+
+def g():
+    pass
+"))))
+
+(ert-deftest python-hideshow-hide-all-1 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "if 0:
+
+def f():
+"))))
+
+(ert-deftest python-hideshow-hide-all-2 ()
+  "Should hide comments."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-all-3 ()
+  "Should not hide comments when `hs-hide-comments-when-hiding-all' is nil."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (let ((hs-hide-comments-when-hiding-all nil))
+     (hs-hide-all))
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-block-1 ()
+  "Should hide current block."
+  (python-tests-with-temp-buffer
+   "
+if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (python-tests-look-at "ddd")
+   (forward-line)
+   (hs-hide-block)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+if 0:
+
+def f():
+    pass
+"))))
+
 
 (ert-deftest python-tests--python-nav-end-of-statement--infloop ()
   "Checks that `python-nav-end-of-statement' doesn't infloop in a

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-15 15:00                   ` kobarity
@ 2022-08-15 17:16                     ` Dima Kogan
  2022-08-16  2:28                       ` kobarity
  2022-08-17 10:45                     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 21+ messages in thread
From: Dima Kogan @ 2022-08-15 17:16 UTC (permalink / raw)
  To: kobarity; +Cc: Lars Ingebrigtsen, Augusto Stoffel, 56635

Thanks for working on this. I'll try it out in a bit. How do you usually
test it?

The main modification is in

  (define-derived-mode python-mode prog-mode "Python" ...
    ...
    (add-to-list
     'hs-special-modes-alist
     `(python-mode
       ....

Re-evaluating the (define-derived-mode ...) form doesn't do it,
obviously. I've tried to manually update hs-special-modes-alist, but
that doesn't work right either. What do you do? Ideally I'd like to drop
something into my ~/.emacs to use the patched logic to long-term test
this, but I haven't been able to figure it out.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-15 17:16                     ` Dima Kogan
@ 2022-08-16  2:28                       ` kobarity
  0 siblings, 0 replies; 21+ messages in thread
From: kobarity @ 2022-08-16  2:28 UTC (permalink / raw)
  To: Dima Kogan; +Cc: Lars Ingebrigtsen, Augusto Stoffel, 56635

Dima Kogan <dima@secretsauce.net> wrote:
> Thanks for working on this. I'll try it out in a bit. How do you usually
> test it?

You need the latest source code (master branch) to apply the patches.
Here are how to test them:

- patch -p1 < fix-56635-hideshow.patch
- patch -p1 < fix-56635-python.patch
- emacs -Q
- M-x load-file "patched python.el"
- M-x load-file "patched hideshow.el"
- M-x find-file "Python file to test"
- M-x hs-minor-mode

If you want to load patched files in ~/.emacs, following settings
should suffice:

(load "/somewhere patched/python.el")
(load "/somewhere patched/hideshow.el")

As for ERTs, I'm testing them with the following command in the
patched source code tree:

src/emacs -batch -l ert -l lisp/progmodes/hideshow.el -l
lisp/progmodes/python.el -l test/lisp/progmodes/python-tests.el -l
test/lisp/progmodes/hideshow-tests.el --eval '(let
((python-tests-shell-interpreter "python3"))
(ert-run-tests-batch-and-exit))'

I'm looking forward to your feedback.

Regards,





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-15 15:00                   ` kobarity
  2022-08-15 17:16                     ` Dima Kogan
@ 2022-08-17 10:45                     ` Lars Ingebrigtsen
  2022-08-17 11:11                       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-17 10:45 UTC (permalink / raw)
  To: kobarity; +Cc: Augusto Stoffel, Dima Kogan, 56635

kobarity <kobarity@gmail.com> writes:

> I fixed some issues in the Python patch.  Please note that the fix for
> #57223 (merged recently) is also needed for all ERTs to pass.

Thanks; pushed to Emacs 29.






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-17 10:45                     ` Lars Ingebrigtsen
@ 2022-08-17 11:11                       ` Lars Ingebrigtsen
  2022-08-17 11:32                         ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-17 11:11 UTC (permalink / raw)
  To: kobarity; +Cc: Augusto Stoffel, Dima Kogan, 56635

Lars Ingebrigtsen <larsi@gnus.org> writes:

>
> kobarity <kobarity@gmail.com> writes:
>
>> I fixed some issues in the Python patch.  Please note that the fix for
>> #57223 (merged recently) is also needed for all ERTs to pass.
>
> Thanks; pushed to Emacs 29.

This led to test failures I didn't notice before pushing:

4 unexpected results:
   FAILED  python-hideshow-hide-all-2
   FAILED  python-hideshow-hide-block-1
   FAILED  python-hideshow-hide-levels-1
   FAILED  python-hideshow-hide-levels-2

So I've now reverted the change.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-17 11:11                       ` Lars Ingebrigtsen
@ 2022-08-17 11:32                         ` kobarity
  2022-08-24  0:39                           ` Dima Kogan
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-08-17 11:32 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Augusto Stoffel, Dima Kogan, 56635

Lars Ingebrigtsen <larsi@gnus.org> wrote:
> This led to test failures I didn't notice before pushing:
>
> 4 unexpected results:
>    FAILED  python-hideshow-hide-all-2
>    FAILED  python-hideshow-hide-block-1
>    FAILED  python-hideshow-hide-levels-1
>    FAILED  python-hideshow-hide-levels-2
>
> So I've now reverted the change.

Sorry for the confusion.  It depends on the patch for hideshow.el
(fix-56635-hideshow.patch) presented in:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56635#32

All tests should pass if both patches are applied.  However, I think
we can wait for feedbacks from Dima and (hopefully) others.

Best Regards,





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-17 11:32                         ` kobarity
@ 2022-08-24  0:39                           ` Dima Kogan
  2022-08-24 10:27                             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 21+ messages in thread
From: Dima Kogan @ 2022-08-24  0:39 UTC (permalink / raw)
  To: kobarity; +Cc: Lars Ingebrigtsen, Augusto Stoffel, 56635

Hi. Thanks for working on this. I just tried this out, and it appears to
work well! I think we can merge this.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-24  0:39                           ` Dima Kogan
@ 2022-08-24 10:27                             ` Lars Ingebrigtsen
  2022-08-24 14:15                               ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-24 10:27 UTC (permalink / raw)
  To: Dima Kogan; +Cc: kobarity, Augusto Stoffel, 56635

Dima Kogan <dima@secretsauce.net> writes:

> Hi. Thanks for working on this. I just tried this out, and it appears to
> work well! I think we can merge this.

Since this depends on two patch sets (and there are different ones?),
could you (kobarity, that is) repost the patches (in the order they
should be applied), and I'll get them pushed.







^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-24 10:27                             ` Lars Ingebrigtsen
@ 2022-08-24 14:15                               ` kobarity
  2022-08-25 12:31                                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 21+ messages in thread
From: kobarity @ 2022-08-24 14:15 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Augusto Stoffel, Dima Kogan, 56635

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Dima Kogan <dima@secretsauce.net> writes:
>
> > Hi. Thanks for working on this. I just tried this out, and it appears to
> > work well! I think we can merge this.
>
> Since this depends on two patch sets (and there are different ones?),
> could you (kobarity, that is) repost the patches (in the order they
> should be applied), and I'll get them pushed.

Dima, thank you for your feedback!

Lars, attached are the revised and rebased patches.  They should be
applied in the order:

- 1-hideshow.patch
- 2-python.patch

No other patches are required.  Thank you for your cooperation.

Best regards,

[-- Attachment #2: 1-hideshow.patch --]
[-- Type: application/octet-stream, Size: 15527 bytes --]

commit cdfd5a64fb2c97684cb92b0ae84aada84a30ad97
Author: kobarity <kobarity@gmail.com>
Date:   Wed Aug 24 22:26:47 2022 +0900

    Extend `hs-special-modes-alist' for languages such as Python
    
    * lisp/progmodes/hideshow.el (hs-special-modes-alist): Add
    elements FIND-BLOCK-BEGINNING-FUNC, FIND-NEXT-BLOCK-FUNC, and
    LOOKING-AT-BLOCK-START-P-FUNC.
    (hs-find-block-beginning-func): New variable to hold
    FIND-BLOCK-BEGINNING-FUNC.
    (hs-find-next-block-func): New variable to hold
    FIND-NEXT-BLOCK-FUNC.
    (hs-looking-at-block-start-p-func): New variable to hold
    LOOKING-AT-BLOCK-START-P-FUNC.
    (hs-grok-mode-type): Set new variables from
    `hs-special-modes-alist'.
    (hs-find-next-block): New function.
    (Misc.): Update callers of the above functions.
    
    * test/lisp/progmodes/hideshow-tests.el: New test file (bug#56635).

diff --git a/lisp/progmodes/hideshow.el b/lisp/progmodes/hideshow.el
index f574ec84fb..c0796fc2ee 100644
--- a/lisp/progmodes/hideshow.el
+++ b/lisp/progmodes/hideshow.el
@@ -267,7 +267,9 @@ hs-special-modes-alist
     ))
   "Alist for initializing the hideshow variables for different modes.
 Each element has the form
-  (MODE START END COMMENT-START FORWARD-SEXP-FUNC ADJUST-BEG-FUNC).
+  (MODE START END COMMENT-START FORWARD-SEXP-FUNC ADJUST-BEG-FUNC
+   FIND-BLOCK-BEGINNING-FUNC FIND-NEXT-BLOCK-FUNC
+   LOOKING-AT-BLOCK-START-P-FUNC).
 
 If non-nil, hideshow will use these values as regexps to define blocks
 and comments, respectively for major mode MODE.
@@ -288,6 +290,15 @@ hs-special-modes-alist
 See the documentation for `hs-adjust-block-beginning' to see what is the
 use of ADJUST-BEG-FUNC.
 
+See the documentation for `hs-find-block-beginning-func' to see
+what is the use of FIND-BLOCK-BEGINNING-FUNC.
+
+See the documentation for `hs-find-next-block-func' to see what
+is the use of FIND-NEXT-BLOCK-FUNC.
+
+See the documentation for `hs-looking-at-block-start-p-func' to
+see what is the use of LOOKING-AT-BLOCK-START-P-FUNC.
+
 If any of the elements is left nil or omitted, hideshow tries to guess
 appropriate values.  The regexps should not contain leading or trailing
 whitespace.  Case does not matter.")
@@ -433,6 +444,39 @@ hs-adjust-block-beginning
 
 See `hs-c-like-adjust-block-beginning' for an example of using this.")
 
+(defvar-local hs-find-block-beginning-func #'hs-find-block-beginning
+  "Function used to do `hs-find-block-beginning'.
+It should reposition point at the beginning of the current block
+and return point, or nil if original point was not in a block.
+
+Specifying this function is necessary for languages such as
+Python, where regexp search and `syntax-ppss' check is not enough
+to find the beginning of the current block.")
+
+(defvar-local hs-find-next-block-func #'hs-find-next-block
+  "Function used to do `hs-find-next-block'.
+It should reposition point at next block start.
+
+It is called with three arguments REGEXP, MAXP, and COMMENTS.
+REGEXP is a regexp representing block start.  When block start is
+found, `match-data' should be set using REGEXP.  MAXP is a buffer
+position that bounds the search.  When COMMENTS is nil, comments
+should be skipped.  When COMMENTS is not nil, REGEXP matches not
+only beginning of a block but also beginning of a comment.  In
+this case, the function should find nearest block or comment.
+
+Specifying this function is necessary for languages such as
+Python, where regexp search is not enough to find the beginning
+of the next block.")
+
+(defvar-local hs-looking-at-block-start-p-func #'hs-looking-at-block-start-p
+  "Function used to do `hs-looking-at-block-start-p'.
+It should return non-nil if the point is at the block start.
+
+Specifying this function is necessary for languages such as
+Python, where `looking-at' and `syntax-ppss' check is not enough
+to check if the point is at the block start.")
+
 (defvar hs-headline nil
   "Text of the line where a hidden block begins, set during isearch.
 You can display this in the mode line by adding the symbol `hs-headline'
@@ -565,7 +609,7 @@ hs-hide-block-at-point
 and then further adjusted to be at the end of the line."
   (if comment-reg
       (hs-hide-comment-region (car comment-reg) (cadr comment-reg) end)
-    (when (hs-looking-at-block-start-p)
+    (when (funcall hs-looking-at-block-start-p-func)
       (let ((mdata (match-data t))
             (header-end (match-end 0))
             p q ov)
@@ -672,7 +716,14 @@ hs-grok-mode-type
                                                      0 (1- (match-end 0)))
                                         c-start-regexp)))
               hs-forward-sexp-func (or (nth 4 lookup) #'forward-sexp)
-              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)))
+              hs-adjust-block-beginning (or (nth 5 lookup) #'identity)
+              hs-find-block-beginning-func (or (nth 6 lookup)
+                                               #'hs-find-block-beginning)
+              hs-find-next-block-func (or (nth 7 lookup)
+                                          #'hs-find-next-block)
+              hs-looking-at-block-start-p-func
+              (or (nth 8 lookup)
+                  #'hs-looking-at-block-start-p)))
     (setq hs-minor-mode nil)
     (error "%s Mode doesn't support Hideshow Minor Mode"
            (format-mode-line mode-name))))
@@ -683,7 +734,7 @@ hs-find-block-beginning
   (let ((done nil)
         (here (point)))
     ;; look if current line is block start
-    (if (hs-looking-at-block-start-p)
+    (if (funcall hs-looking-at-block-start-p-func)
         (point)
       ;; look backward for the start of a block that contains the cursor
       (while (and (re-search-backward hs-block-start-regexp nil t)
@@ -698,19 +749,25 @@ hs-find-block-beginning
         (goto-char here)
         nil))))
 
+(defun hs-find-next-block (regexp maxp comments)
+  "Reposition point at next block-start.
+Skip comments if COMMENTS is nil, and search for REGEXP in
+region (point MAXP)."
+  (when (not comments)
+    (forward-comment (point-max)))
+  (and (< (point) maxp)
+       (re-search-forward regexp maxp t)))
+
 (defun hs-hide-level-recursive (arg minp maxp)
   "Recursively hide blocks ARG levels below point in region (MINP MAXP)."
-  (when (hs-find-block-beginning)
+  (when (funcall hs-find-block-beginning-func)
     (setq minp (1+ (point)))
     (funcall hs-forward-sexp-func 1)
     (setq maxp (1- (point))))
   (unless hs-allow-nesting
     (hs-discard-overlays minp maxp))
   (goto-char minp)
-  (while (progn
-           (forward-comment (buffer-size))
-           (and (< (point) maxp)
-                (re-search-forward hs-block-start-regexp maxp t)))
+  (while (funcall hs-find-next-block-func hs-block-start-regexp maxp nil)
     (when (save-match-data
 	    (not (nth 8 (syntax-ppss)))) ; not inside comments or strings
       (if (> arg 1)
@@ -747,8 +804,8 @@ hs-already-hidden-p
           (goto-char (nth 0 c-reg))
         (end-of-line)
         (when (and (not c-reg)
-                   (hs-find-block-beginning)
-		   (hs-looking-at-block-start-p))
+                   (funcall hs-find-block-beginning-func)
+		   (funcall hs-looking-at-block-start-p-func))
           ;; point is inside a block
           (goto-char (match-end 0)))))
     (end-of-line)
@@ -790,10 +847,8 @@ hs-hide-all
                                    hs-c-start-regexp
                                    "\\)")
                          ""))))
-       (while (progn
-                (unless hs-hide-comments-when-hiding-all
-                  (forward-comment (point-max)))
-                (re-search-forward re (point-max) t))
+       (while (funcall hs-find-next-block-func re (point-max)
+                       hs-hide-comments-when-hiding-all)
          (if (match-beginning 1)
              ;; We have found a block beginning.
              (progn
@@ -838,8 +893,8 @@ hs-hide-block
                       (<= (count-lines (car c-reg) (nth 1 c-reg)) 1)))
        (message "(not enough comment lines to hide)"))
       ((or c-reg
-	   (hs-looking-at-block-start-p)
-           (hs-find-block-beginning))
+	   (funcall hs-looking-at-block-start-p-func)
+           (funcall hs-find-block-beginning-func))
        (hs-hide-block-at-point end c-reg)
        (run-hooks 'hs-hide-hook))))))
 
@@ -868,9 +923,9 @@ hs-show-block
              (when (car c-reg)
                (setq p (car c-reg)
                      q (cadr c-reg))))
-            ((and (hs-find-block-beginning)
+            ((and (funcall hs-find-block-beginning-func)
                   ;; ugh, fresh match-data
-                  (hs-looking-at-block-start-p))
+                  (funcall hs-looking-at-block-start-p-func))
              (setq p (point)
                    q (progn (hs-forward-sexp (match-data t) 1) (point)))))
       (when (and p q)
diff --git a/test/lisp/progmodes/hideshow-tests.el b/test/lisp/progmodes/hideshow-tests.el
new file mode 100644
index 0000000000..ee2a0c7c4c
--- /dev/null
+++ b/test/lisp/progmodes/hideshow-tests.el
@@ -0,0 +1,268 @@
+;;; hideshow-tests.el --- Test suite for hideshow.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'hideshow)
+
+;; Dependencies for testing:
+(require 'cc-mode)
+
+
+(defmacro hideshow-tests-with-temp-buffer (mode contents &rest body)
+  "Create a `hs-minor-mode' enabled MODE temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (,mode)
+     (hs-minor-mode 1)
+     (insert ,contents)
+     (goto-char (point-min))
+     ,@body))
+
+(defun hideshow-tests-look-at (string &optional num restore-point)
+  "Move point at beginning of STRING in the current buffer.
+Optional argument NUM defaults to 1 and is an integer indicating
+how many occurrences must be found, when positive the search is
+done forwards, otherwise backwards.  When RESTORE-POINT is
+non-nil the point is not moved but the position found is still
+returned.  When searching forward and point is already looking at
+STRING, it is skipped so the next STRING occurrence is selected."
+  (let* ((num (or num 1))
+         (starting-point (point))
+         (string (regexp-quote string))
+         (search-fn (if (> num 0) #'re-search-forward #'re-search-backward))
+         (deinc-fn (if (> num 0) #'1- #'1+))
+         (found-point))
+    (prog2
+        (catch 'exit
+          (while (not (= num 0))
+            (when (and (> num 0)
+                       (looking-at string))
+              ;; Moving forward and already looking at STRING, skip it.
+              (forward-char (length (match-string-no-properties 0))))
+            (and (not (funcall search-fn string nil t))
+                 (throw 'exit t))
+            (when (> num 0)
+              ;; `re-search-forward' leaves point at the end of the
+              ;; occurrence, move back so point is at the beginning
+              ;; instead.
+              (forward-char (- (length (match-string-no-properties 0)))))
+            (setq
+             num (funcall deinc-fn num)
+             found-point (point))))
+        found-point
+      (and restore-point (goto-char starting-point)))))
+
+(defun hideshow-tests-visible-string (&optional min max)
+  "Return the buffer string excluding invisible overlays.
+Argument MIN and MAX delimit the region to be returned and
+default to `point-min' and `point-max' respectively."
+  (let* ((min (or min (point-min)))
+         (max (or max (point-max)))
+         (buffer-contents (buffer-substring-no-properties min max))
+         (overlays
+          (sort (overlays-in min max)
+                (lambda (a b)
+                  (let ((overlay-end-a (overlay-end a))
+                        (overlay-end-b (overlay-end b)))
+                    (> overlay-end-a overlay-end-b))))))
+    (with-temp-buffer
+      (insert buffer-contents)
+      (dolist (overlay overlays)
+        (if (overlay-get overlay 'invisible)
+            (delete-region (overlay-start overlay)
+                           (overlay-end overlay))))
+      (buffer-substring-no-properties (point-min) (point-max)))))
+
+(ert-deftest hideshow-hide-block-1 ()
+  "Should hide current block."
+  (let ((contents "
+int
+main()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (hideshow-tests-look-at "printf")
+     (hs-hide-block)
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+int
+main()
+{}
+"))
+     (hs-show-block)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-all-1 ()
+  "Should hide all blocks and comments."
+  (let ((contents "
+/*
+   Comments
+*/
+
+int
+main()
+{
+  sub();
+}
+
+void
+sub()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (hs-hide-all)
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+/*
+
+int
+main()
+{}
+
+void
+sub()
+{}
+"))
+     (hs-show-all)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-all-2 ()
+  "Should not hide comments when `hs-hide-comments-when-hiding-all' is nil."
+  (let ((contents "
+/*
+   Comments
+*/
+
+int
+main()
+{
+  sub();
+}
+
+void
+sub()
+{
+  printf(\"Hello\\n\");
+}
+"))
+    (hideshow-tests-with-temp-buffer
+     c-mode
+     contents
+     (let ((hs-hide-comments-when-hiding-all nil))
+       (hs-hide-all))
+     (should (string=
+              (hideshow-tests-visible-string)
+              "
+/*
+   Comments
+*/
+
+int
+main()
+{}
+
+void
+sub()
+{}
+"))
+     (hs-show-all)
+     (should (string= (hideshow-tests-visible-string) contents)))))
+
+(ert-deftest hideshow-hide-level-1 ()
+  "Should hide 1st level blocks."
+  (hideshow-tests-with-temp-buffer
+   c-mode
+   "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {
+    printf(\"Hello\\n\");
+  }
+}
+"
+   (hs-hide-level 1)
+   (should (string=
+            (hideshow-tests-visible-string)
+            "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{}
+"))))
+
+(ert-deftest hideshow-hide-level-2 ()
+  "Should hide 2nd level blocks."
+  (hideshow-tests-with-temp-buffer
+   c-mode
+   "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {
+    printf(\"Hello\\n\");
+  }
+}
+"
+   (hs-hide-level 2)
+   (should (string=
+            (hideshow-tests-visible-string)
+            "
+/*
+   Comments
+*/
+
+int
+main(int argc, char **argv)
+{
+  if (argc > 1) {}
+}
+"))))
+
+(provide 'hideshow-tests)
+
+;;; hideshow-tests.el ends here

[-- Attachment #3: 2-python.patch --]
[-- Type: application/octet-stream, Size: 9299 bytes --]

commit 1b778fe2e0220b946f0c1d7e510c4d10d0351977
Author: kobarity <kobarity@gmail.com>
Date:   Wed Aug 24 22:27:09 2022 +0900

    Add Python blocks support for hideshow
    
    * lisp/progmodes/python.el (python-nav-beginning-of-block-regexp):
    New variable.
    (python-hideshow-forward-sexp-function): Change to call
    `python-nav-end-of-block'.
    (python-hideshow-find-next-block): New function to be used as
    FIND-NEXT-BLOCK-FUNC in `hs-special-modes-alist'.
    (python-info-looking-at-beginning-of-block): New function to be
    used as LOOKING-AT-BLOCK-START-P-FUNC in `hs-special-modes-alist'.
    (python-mode): Change settings of `hs-special-modes-alist'.
    
    * test/lisp/progmodes/python-tests.el
    (python-hideshow-hide-levels-1): Fix to keep empty lines.
    (python-info-looking-at-beginning-of-block-1)
    (python-hideshow-hide-levels-3, python-hideshow-hide-levels-4)
    (python-hideshow-hide-all-1, python-hideshow-hide-all-2)
    (python-hideshow-hide-all-3, python-hideshow-hide-block-1): New
    tests (bug#56635).

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index e1347754c4..d3ffc2db2c 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1524,6 +1524,10 @@ python-nav-beginning-of-defun-regexp
 The name of the defun should be grouped so it can be retrieved
 via `match-string'.")
 
+(defvar python-nav-beginning-of-block-regexp
+  (python-rx line-start (* space) block-start)
+  "Regexp matching block start.")
+
 (defun python-nav--beginning-of-defun (&optional arg)
   "Internal implementation of `python-nav-beginning-of-defun'.
 With positive ARG search backwards, else search forwards."
@@ -4916,9 +4920,37 @@ python-describe-at-point
 (defun python-hideshow-forward-sexp-function (_arg)
   "Python specific `forward-sexp' function for `hs-minor-mode'.
 Argument ARG is ignored."
-  (python-nav-end-of-defun)
-  (unless (python-info-current-line-empty-p)
-    (backward-char)))
+  (python-nav-end-of-block))
+
+(defun python-hideshow-find-next-block (regexp maxp comments)
+  "Python specific `hs-find-next-block' function for `hs-minor-mode'.
+Call `python-nav-forward-block' to find next block and check if
+block-start ends within MAXP.  If COMMENTS is not nil, comments
+are also searched.  REGEXP is passed to `looking-at' to set
+`match-data'."
+  (let* ((next-block (save-excursion
+                       (or (and
+                            (python-info-looking-at-beginning-of-block)
+                            (re-search-forward
+                             (python-rx block-start) maxp t))
+                           (and (python-nav-forward-block)
+                                (< (point) maxp)
+                                (re-search-forward
+                                 (python-rx block-start) maxp t))
+                           (1+ maxp))))
+         (next-comment
+          (or (when comments
+                (save-excursion
+                  (cl-loop while (re-search-forward "#" maxp t)
+                           if (python-syntax-context 'comment)
+                           return (point))))
+              (1+ maxp)))
+         (next-block-or-comment (min next-block next-comment)))
+    (when (<= next-block-or-comment maxp)
+      (goto-char next-block-or-comment)
+      (save-excursion
+        (beginning-of-line)
+        (looking-at regexp)))))
 
 \f
 ;;; Imenu
@@ -5415,6 +5447,16 @@ python-info-looking-at-beginning-of-defun
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
+(defun python-info-looking-at-beginning-of-block ()
+  "Check if point is at the beginning of block."
+  (let ((pos (point)))
+    (save-excursion
+      (python-nav-beginning-of-statement)
+      (beginning-of-line)
+      (and
+       (<= (point) pos (+ (point) (current-indentation)))
+       (looking-at python-nav-beginning-of-block-regexp)))))
+
 (defun python-info-current-line-comment-p ()
   "Return non-nil if current line is a comment line."
   (char-equal
@@ -5870,14 +5912,17 @@ python-mode
 
   (add-to-list
    'hs-special-modes-alist
-   '(python-mode
-     "\\s-*\\_<\\(?:def\\|class\\)\\_>"
+   `(python-mode
+     ,python-nav-beginning-of-block-regexp
      ;; Use the empty string as end regexp so it doesn't default to
      ;; "\\s)".  This way parens at end of defun are properly hidden.
      ""
      "#"
      python-hideshow-forward-sexp-function
-     nil))
+     nil
+     python-nav-beginning-of-block
+     python-hideshow-find-next-block
+     python-info-looking-at-beginning-of-block))
 
   (setq-local outline-regexp (python-rx (* space) block-start))
   (setq-local outline-level
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 12ac871fdf..906f7eca7d 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5730,6 +5730,39 @@ python-info-looking-at-beginning-of-defun-2
    (should (not (python-info-looking-at-beginning-of-defun)))
    (should (not (python-info-looking-at-beginning-of-defun nil t)))))
 
+(ert-deftest python-info-looking-at-beginning-of-block-1 ()
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if True:
+        pass
+    l = [x * 2
+         for x in range(5)
+         if x < 3]
+# if False:
+\"\"\"
+if 0:
+\"\"\"
+"
+   (python-tests-look-at "def f():")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if True:")
+   (should (python-info-looking-at-beginning-of-block))
+   (forward-char)
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (beginning-of-line)
+   (should (python-info-looking-at-beginning-of-block))
+   (python-tests-look-at "for x")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if x < 3")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if False:")
+   (should (not (python-info-looking-at-beginning-of-block)))
+   (python-tests-look-at "if 0:")
+   (should (not (python-info-looking-at-beginning-of-block)))))
+
 (ert-deftest python-info-current-line-comment-p-1 ()
   (python-tests-with-temp-buffer
    "
@@ -6183,8 +6216,11 @@ python-hideshow-hide-levels-1
 class SomeClass:
 
     def __init__(self, arg, kwarg=1):
+
     def filter(self, nums):
-    def __str__(self):"))))
+
+    def __str__(self):
+"))))
       (or enabled (hs-minor-mode -1)))))
 
 (ert-deftest python-hideshow-hide-levels-2 ()
@@ -6230,6 +6266,165 @@ python-hideshow-hide-levels-2
 "))))
       (or enabled (hs-minor-mode -1)))))
 
+(ert-deftest python-hideshow-hide-levels-3 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+
+def g():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 1)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+
+def g():
+"))))
+
+(ert-deftest python-hideshow-hide-levels-4 ()
+  "Should hide 2nd level block."
+  (python-tests-with-temp-buffer
+   "
+def f():
+    if 0:
+        l = [i for i in range(5)
+             if i < 3]
+        abc = o.match(1, 2, 3)
+
+def g():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-level 2)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+def f():
+    if 0:
+
+def g():
+    pass
+"))))
+
+(ert-deftest python-hideshow-hide-all-1 ()
+  "Should hide all blocks."
+  (python-tests-with-temp-buffer
+   "if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "if 0:
+
+def f():
+"))))
+
+(ert-deftest python-hideshow-hide-all-2 ()
+  "Should hide comments."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (hs-hide-all)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-all-3 ()
+  "Should not hide comments when `hs-hide-comments-when-hiding-all' is nil."
+  (python-tests-with-temp-buffer
+   "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"
+   (hs-minor-mode 1)
+   (let ((hs-hide-comments-when-hiding-all nil))
+     (hs-hide-all))
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+# Multi line
+# comment
+
+\"\"\"
+# Multi line
+# string
+\"\"\"
+"))))
+
+(ert-deftest python-hideshow-hide-block-1 ()
+  "Should hide current block."
+  (python-tests-with-temp-buffer
+   "
+if 0:
+
+    aaa
+    l = [i for i in range(5)
+         if i < 3]
+    ccc
+    abc = o.match(1, 2, 3)
+    ddd
+
+def f():
+    pass
+"
+   (hs-minor-mode 1)
+   (python-tests-look-at "ddd")
+   (forward-line)
+   (hs-hide-block)
+   (should
+    (string=
+     (python-tests-visible-string)
+     "
+if 0:
+
+def f():
+    pass
+"))))
+
 
 (ert-deftest python-tests--python-nav-end-of-statement--infloop ()
   "Checks that `python-nav-end-of-statement' doesn't infloop in a

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-24 14:15                               ` kobarity
@ 2022-08-25 12:31                                 ` Lars Ingebrigtsen
  2022-08-25 13:39                                   ` kobarity
  0 siblings, 1 reply; 21+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-25 12:31 UTC (permalink / raw)
  To: kobarity; +Cc: Augusto Stoffel, Dima Kogan, 56635

kobarity <kobarity@gmail.com> writes:

> Lars, attached are the revised and rebased patches.  They should be
> applied in the order:
>
> - 1-hideshow.patch
> - 2-python.patch
>
> No other patches are required.  Thank you for your cooperation.

Thanks; pushed to Emacs 29.  I had to apply the patches "by hand"
instead of with "git am" because "git am" complained:

---
Nonempty second line in commit message
Commit aborted; please see the file CONTRIBUTE
---

I'm not quite sure what that refers to -- the second line looked pretty
empty to me (except some spaces).  But even removing those spaces didn't
help.






^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks
  2022-08-25 12:31                                 ` Lars Ingebrigtsen
@ 2022-08-25 13:39                                   ` kobarity
  0 siblings, 0 replies; 21+ messages in thread
From: kobarity @ 2022-08-25 13:39 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Augusto Stoffel, Dima Kogan, 56635

Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Thanks; pushed to Emacs 29.  I had to apply the patches "by hand"
> instead of with "git am" because "git am" complained:

Thank you for applying the patches, and sorry for the inconvenience.
I'm going to review my email environment.

Regards,





^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-08-25 13:39 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 22:20 bug#56635: 29.0.50; [PATCH] hide-show in python-mode supports ONLY function and class blocks Dima Kogan
2022-07-23  7:58 ` Lars Ingebrigtsen
2022-07-23 15:51   ` kobarity
2022-07-23 20:41     ` Dima Kogan
2022-07-24  0:57       ` kobarity
2022-07-31 10:18         ` kobarity
2022-07-31 19:04           ` Dima Kogan
2022-08-07 14:37             ` kobarity
2022-08-08 15:16               ` Augusto Stoffel
2022-08-13 12:36                 ` kobarity
2022-08-15 15:00                   ` kobarity
2022-08-15 17:16                     ` Dima Kogan
2022-08-16  2:28                       ` kobarity
2022-08-17 10:45                     ` Lars Ingebrigtsen
2022-08-17 11:11                       ` Lars Ingebrigtsen
2022-08-17 11:32                         ` kobarity
2022-08-24  0:39                           ` Dima Kogan
2022-08-24 10:27                             ` Lars Ingebrigtsen
2022-08-24 14:15                               ` kobarity
2022-08-25 12:31                                 ` Lars Ingebrigtsen
2022-08-25 13:39                                   ` kobarity

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).