Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

outline-minor-mode doesn't behave correctly with clojure-mode #550

Closed
j-cr opened this issue Dec 25, 2019 · 5 comments
Closed

outline-minor-mode doesn't behave correctly with clojure-mode #550

j-cr opened this issue Dec 25, 2019 · 5 comments

Comments

@j-cr
Copy link
Contributor

j-cr commented Dec 25, 2019

TL;DR: in clojure-mode-variables, the line
(setq-local outline-regexp ";;;\\(;* [^ \t\n]\\)\\|(")
is likely incorrect; replacing it with (setq-local outline-regexp ";;;;* ") works for me.

Expected behavior

Do M-x outline-minor-mode in a clojure buffer; the command outline-insert-heading should behave correctly.

In particular, call to outline-insert-heading on the first line of each pair should result in the second line:

;;; heading:
;;; 

;;;; h2:
;;;; 

(foo)
;;; 

Actual behavior

;;; heading:
;;; h 

;;;; h2:
;;;; h 

(foo)
( 

Steps to reproduce the problem

See above, tldr; M-x outline-minor-mode, then M-x outline-insert-heading.

Environment & Version information

clojure-mode version information

clojure-mode (version 5.11.0)

Emacs version

25.3.1

@bbatsov
Copy link
Member

bbatsov commented Jan 25, 2020

Seems something's wrong with the current regexp indeed, although it doesn't seem obviously incorrect. The intention seems to be to check for an extra ; or some whitespace after ;;; which makes sense to me. \\|( looks a bit weird and I'm not sure why this was added.

@bbatsov
Copy link
Member

bbatsov commented Jan 25, 2020

Btw, it seems we copied this from lisp-mode, as there we have:

  (setq-local outline-regexp ";;;\\(;* [^ \t\n]\\|###autoload\\)\\|(")

@j-cr
Copy link
Contributor Author

j-cr commented Jan 26, 2020

Re \\|(: if I understand correctly, the intent was to treat top-level expressions as headings, so e.g. the visibility of defuns could be cycled as if the were headings.

But there are packages (bicycle) that solve this problem by treating top-level exprs explicitly (bicycle knows how to hide them independently of what outline-regexp is), so I think it's not a problem really: if the user wants to hide top-level exprs, he can install bicycle or a similar package.

Re ;* [^ \t\n]: the intention seems to require a space and then some non-whitespace text in order to treat triple semicolon as a heading, thus:

;;;                        -- not a heading
;;;foo                     -- not a heading
;;; foo                    -- heading

but it means that the "space and then a non-whitespace character" part is included in the match, and outline-insert-heading uses that match for the heading prefix, thus producing this behavior:

;;; heading:           -- M-x outline-insert-heading =>
;;; h 

So, if we do (setq-local outline-regexp ";;;;* ") instead, the behavior will be slightly different from lisp-mode - namely, an empty line prefixed with ;;; will be considered a heading - but I don't think that's a problem.

j-cr added a commit to j-cr/clojure-mode that referenced this issue Jan 26, 2020
Update outline-regexp so outline-insert-heading behaves correctly.
@bbatsov
Copy link
Member

bbatsov commented Mar 21, 2020

Shouldn't we at least expect some characters (e.g. letters/numbers/punctuation) after the ;;;? Empty headings would be pretty weird IMO.

@j-cr
Copy link
Contributor Author

j-cr commented Mar 22, 2020

For example org-mode treats empty headings (* ) as headings still, so I think it's ok (or maybe even better that lisp-mode in terms of consistency, depending on your viewpoint).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants