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.

Sign up for GitHub

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

Introduce scala-indent:defition-parameter-scaling-factor #173

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Introduce scala-indent:defition-parameter-scaling-factor #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions scala-mode-indent.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ indentation will be one or two steps depending on context."
:safe #'integerp
:group 'scala)

(defcustom scala-indent:defition-parameter-scaling-factor 1
"Scale the number of spaces used to indent the parameter list
of class/function definitions. See also
https://github.com/hvesalai/emacs-scala-mode/issues/172"
:type 'integer
:safe #'integerp
:group 'scala)

(defcustom scala-indent:indent-value-expression nil
"Whether or not to indent multi-line value expressions, with
one extra step. When true, indenting will be
Expand Down Expand Up @@ -629,6 +637,9 @@ anchor for calculating block indent for current point (or point
(> (line-number-at-pos) (line-number-at-pos anchor))
(> start (match-beginning 0))))
(+ (* 2 scala-indent:step) lead))
;; optionally double the step for parameter list
((scala-syntax:in-definition-parameter-list anchor start)
(+ (* scala-indent:defition-parameter-scaling-factor scala-indent:step) lead))
;; normal block line
(t (+ scala-indent:step lead)))))

Expand Down
19 changes: 19 additions & 0 deletions scala-mode-syntax.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,25 @@ val a, b = (1, 2)
(scala-syntax:find-brace-equals-or-next)
(scala-syntax:handle-brace-equals-or-next))

(defun scala-syntax:in-definition-parameter-list (anchor start)
"Return t if the (anchor, start) region is the parameter list
of a definition(class, def, etc.)"
(save-excursion
(goto-char anchor)
(when
(or (string= "class" (current-word))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fix the indentation here. You are most probably using tabs, please use spaces only.

(string= "def" (current-word)))
;; try to find (
(while (and (< (point) start)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what you are trying to achieve here. I read the code as follows: you search for (, skip it, then search for ), skip that. Why?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was trying to do is:

  • Confirm I can see "(", so we are inside parameter list.
  • Confirm I couldn't see ")" beyond that, so we are not outside parameter list.

I'm not sure whether there is some assumption I can leverage so simplify that, happy to take them.

Also the approach doesn't really work for case like this because there is more complicated scenario like annoation within for the parameter list, like this

case class Option(
     @arg(
        name="port",
        doc="TCP port to listen to"
     )
     port: Int = 0,
)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok... first some note:

  • you can't be searching for ) because you can't expect the code to be complete by the time you already need to indent it. Hence indentation should never look beyond the line that is being indented.

  • you can't expect the definition keyword to be after anchor. For example, the case

def foo
  (
    bar: Int // anchor for this is at (, not the line with def
  )

(you could of course ignore this for your specific modification)

  • you should take into account that both def's and constructors can have multiple parameter lists.

But then some implementation hints:

  1. check out https://github.com/hvesalai/emacs-scala-mode/blob/master/scala-mode-indent.el#L626

here (nth 1 (syntax-ppss start)) gives The character position of the start of the innermost parenthetical grouping containing the stopping point; nil if none. stopping point being start. You can use this to jump to the opening brace to check that it is (.

  1. after checking for ( you are now outside the parenthesis, so you can use backward-sexp to jump over preceding lists (type lists and parameter lists) and the name to arrive at the keyword.

As an example, consider the following code

private class Foo[X <: List[_]](implicit ev: Evidence)(
    x: String,
    y: Int

Let's assume start is at y on the last line and so anchor woudl be at p on the first line.

Now if you follow the instruction 1 above, you can use that to jump out of the parameter list, i.e. to the last ( on the first line. You check that it is indeed ( and if so, then you use backward-sexp to jump over all lists (i.e. jump as long as you land at (, or [). Now you would be at F. You then jump once more using backward-sexp and you should be at c on the first line. You would then use (looking-at-p "class\\|def") to check that you were indeed inside a definition list.

That's it.

HAVING SAID THAT, now that I read the code, I see scala-syntax:beginning-of-definition, which sounds like exactly what you want. However, it jumps to the start of the line, not the definition work. We should refactor the code a bit to get it to do what you need.

I propose you implement the thing without refactoring scala-syntax:beginning-of-definition to see that the approach actually works, and then I can refactor the code before I merge it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the thing I did with syntax-ppss can also be done with backward-up-list, but then you have to handle the error it throws if you are NOT in the list. I think that's why I've done it the low-level way

(/= (char-after) ?\())
(forward-char))
(when (< (point) start)
;; And we haven't see ) yet.
(while (and (< (point) start)
(/= (char-after) ?\)))
(forward-char))
(= (point) start)))))

(defun scala-syntax:find-brace-equals-or-next ()
(scala-syntax:go-to-pos
(save-excursion
Expand Down