Handle block of blank lines for code folding properly (fixes #1707).#1756
Handle block of blank lines for code folding properly (fixes #1707).#1756torimus wants to merge 3 commits intohaskell:masterfrom
Conversation
purcell
left a comment
There was a problem hiding this comment.
Thanks for this! Looks reasonable - I noted a couple of minor issues.
I wonder what you think of my suggestion in the linked issue that we retain a single blank line?
haskell-collapse.el
Outdated
| (progn (skip-chars-forward "[:blank:]") (point))))) | ||
|
|
||
| (defun haskell-blank-lines-block (cmp dir indent) | ||
| "returns `t' if following sequence of blank lines fits specific indentation level" |
There was a problem hiding this comment.
A quick fix to follow docstring conventions:
| "returns `t' if following sequence of blank lines fits specific indentation level" | |
| "Return non-nil if sequence of blank lines in direction DIR fits given INDENT level." |
haskell-collapse.el
Outdated
| (haskell-blank-line-p))) | ||
| (if (funcall cmp (current-indentation) indent) | ||
| (progn | ||
| (when (= dir 1) (forward-line -1)) |
haskell-collapse.el
Outdated
| (progn | ||
| (when (= dir 1) (forward-line -1)) | ||
| t) | ||
| (progn |
There was a problem hiding this comment.
This progn wrapper is redundant in the else clause of if statements, and it would be more idiomatic to omit it.
Thanks for the correction, fixed in a new commit.
Oh sorry, wasn't aware emacs doesn't automatically sets spaces-only for elisp mode indentation like used in other language modes. Fixed.
Right. I'm not learned in Emacs Lisp, now found out in docs else branch is not limited to a single expression.
Not sure if I follow you correctly, but this PR introduces 3 new failed tests in haskell-collapse-tests.el which all depend how would you expect correct code folding. In my opinion, retained blank line(s) between code blocks at the same indentation level(as the initial) are to be expected. Only collapse them in nested indentation levels. I can "fix" tests appropriately to correspond to aforementioned logic so it will pass. |
…ie. retain block of blank lines followed by same (or lesser) indentation level as the initial one.
Consider context of consequent blank lines, to which code & indentation level it belongs.
Leaves blank lines of outer indentation levels intact.
Fixes issue #1707.