hvesalai / emacs-scala-mode

The definitive scala-mode for emacs
http://ensime.org
GNU General Public License v3.0
362 stars 68 forks source link

Incompatible with aggressive-indent-mode #150

Open Pitometsu opened 6 years ago

Pitometsu commented 6 years ago

https://github.com/Malabarba/aggressive-indent-mode/issues/116

scala-mode eat space symbols at the end of current line on imput when aggressive-indent-mode is enabled. I suppose cause in scala-indent:indent-line.

hvesalai commented 6 years ago

If you have an idea about the exact location of the problem, would it be possible for you to make a PR that fixes it for your use case, but doesn't break it for others?

Pitometsu commented 6 years ago

Well, maybe it's because of scala-indent:fixup-whitespace calling. I'm not sure. You right here: it should be customizable option.

Pitometsu commented 6 years ago

Oh, I see, (scala-lib:delete-trailing-whitespace) in scala-indent:indent-code-line.

Pitometsu commented 6 years ago

@hvesalai here's the simple fix:

1 file changed, 9 insertions(+), 2 deletions(-)
scala-mode-indent.el | 11 +++++++++--

modified   scala-mode-indent.el
@@ -125,6 +125,12 @@ when it is added to an empty line."
   :safe #'booleanp
   :group 'scala)

+(defcustom scala-indent:delete-trailing-whitespace t
+  "When non-nil, trailing whitespaces will be removed on indentation."
+  :type 'boolean
+  :safe #'booleanp
+  :group 'scala)
+
 (defcustom scala-indent:use-javadoc-style nil
   "When non-nil, multi-line comments are indented according to Javadoc
 style (i.e. indented to the first asterisk). This overrides the
@@ -799,9 +805,10 @@ strings"
     (if (eq last-command this-command)
         (scala-indent:toggle-effective-run-on-strategy)
       (scala-indent:reset-effective-run-on-strategy)))
 ;  (message "run-on-strategy is %s" (scala-indent:run-on-strategy))
   (scala-indent:indent-line-to (scala-indent:calculate-indent-for-line))
-  (scala-lib:delete-trailing-whitespace)
+  (when scala-indent:delete-trailing-whitespace
+    (scala-lib:delete-trailing-whitespace))
   (setq scala-indent:previous-indent-pos
         (save-excursion
           (beginning-of-line)

plz, add.

Pitometsu commented 6 years ago

It's a dumb fix: it may have more complex behavior like keep deleting trailing whitespaces on all but current line.

Pitometsu commented 6 years ago

@hvesalai more intelligent I'm sure should be more like this (not checked!):

1 file changed, 33 insertions(+), 5 deletions(-)
scala-mode-indent.el | 38 +++++++++++++++++++++++++++++++++-----

modified   scala-mode-indent.el
@@ -87,11 +87,11 @@ val x = if (foo)
   :group 'scala)

 (defconst scala-indent:eager-strategy 0
-  "See 'scala-indent:run-on-strategy'")
+  "See `scala-indent:run-on-strategy'")
 (defconst scala-indent:operator-strategy 1
-  "See 'scala-indent:run-on-strategy'")
+  "See `scala-indent:run-on-strategy'")
 (defconst scala-indent:reluctant-strategy 2
-  "See 'scala-indent:run-on-strategy'")
+  "See `scala-indent:run-on-strategy'")
 (defconst scala-indent:keywords-only-strategy 3
   "A strategy used internally by indent engine")

@@ -125,6 +125,28 @@ when it is added to an empty line."
   :safe #'booleanp
   :group 'scala)

+(defconst scala-indent:delete-trailing-whitespace-yes 1
+  "See `scala-indent:delete-trailing-whitespace'.")
+(defconst scala-indent:delete-trailing-whitespace-no 0
+  "See `scala-indent:delete-trailing-whitespace'.")
+(defconst scala-indent:delete-trailing-whitespace-smart -1
+  "See `scala-indent:delete-trailing-whitespace'.")
+
+(defcustom scala-indent:delete-trailing-whitespace scala-indent:delete-trailing-whitespace-yes
+  "Whether indentation keep trailing whitespaces.
+
+Possible values are:
+
+    Yes -- trailing whitespaces will be removed on indentation.
+    No -- all trailing whitespaces will be kept.
+    Smart -- trailing whitespaces will be removed on indentation for all lines except the current one."
+  :type `(choice (const :tag "Yes" ,scala-indent:delete-trailing-whitespace-yes)
+                 (const :tag "No" ,scala-indent:delete-trailing-whitespace-no)
+                 (const :tag "Smart" ,scala-indent:delete-trailing-whitespace-smart))
+  :group 'scala)
+
 (defcustom scala-indent:use-javadoc-style nil
   "When non-nil, multi-line comments are indented according to Javadoc
 style (i.e. indented to the first asterisk). This overrides the
@@ -799,9 +821,15 @@ strings"
     (if (eq last-command this-command)
         (scala-indent:toggle-effective-run-on-strategy)
       (scala-indent:reset-effective-run-on-strategy)))
-;  (message "run-on-strategy is %s" (scala-indent:run-on-strategy))
+  ;;  (message "run-on-strategy is %s" (scala-indent:run-on-strategy))
   (scala-indent:indent-line-to (scala-indent:calculate-indent-for-line))
-  (scala-lib:delete-trailing-whitespace)
+  (pcase scala-indent:delete-trailing-whitespace
+    (scala-indent:delete-trailing-whitespace-yes
+     (scala-lib:delete-trailing-whitespace))
+    (scala-indent:delete-trailing-whitespace-no t)
+    (scala-indent:delete-trailing-whitespace-smart
+     (when not-current-line
+       (scala-lib:delete-trailing-whitespace))))
   (setq scala-indent:previous-indent-pos
         (save-excursion
           (beginning-of-line)
Pitometsu commented 6 years ago

But I have no clue, how to implement the not-current-line here..

Pitometsu commented 6 years ago

Maybe we need custom indent-region-function for it.

Pitometsu commented 6 years ago

And even in that case indent-region call 'indent-line-function or even 'indent-region-function inside save-excursion. Don't know how to get the original point from there without dirty hacks like advices on indent-region and line-number-at-pos saving in global scope...

Pitometsu commented 6 years ago

@hvesalai ping. Would u merge it?

Pitometsu commented 6 years ago

@hvesalai anybody here? :-)

Antisune commented 5 years ago

:+1: This is a bug I encountered and was inconvenienced by too. Having a fix pulled in would be nice