WIP: Scala 3 - Githubissues

Kazark commented 3 years ago

Putting this here in case anyone wants to follow my progress and comment or critique.

cchantep commented 3 years ago

Would be nice

Kazark commented 3 years ago

@cchantep yep, I'm working on it as I get time. Good news is if you use mostly the old syntax for now, the mode is up to date enough to be reasonable to use with Scala 3. It's going to be a while before there is a lot of code on newly converted codebases that is thoroughly embracing Scala 3 idioms. Hopefully by then I will have this done. My team is going to adopt quickly, so I will have incentive.

Kazark commented 3 years ago

I've turned off Metals, and now I notice that I get these warnings without Metals:

Is this arising from this package? Or is there something going on here with Flycheck or something? (Emacs n00b questions, I know.)

Kazark commented 3 years ago

Ah yes, I discovered that it is a Flycheck acting as a minor mode. The question is, where is Flycheck getting its syntactic knowledge?

Kazark commented 3 years ago

Ah, it is getting it from scalac, so I just have to set that to a new path. But the annoying new scalac always uses ANSI color, without trying to detect whether it's in a terminal, and confuses Flycheck. 🤦‍♂️

smarter commented 3 years ago

You can run the compiler with -color:never, but the compiler output is not guaranteed to be stable (and neither is the scala 2 compiler output for that matter). (And we do run terminal detection but I guess it's not working in your case for some reason.)

Kazark commented 3 years ago

@smarter ah, thank you. BTW enjoyed hearing from you at the release party just now.

Kazark commented 3 years ago

@smarter for further context, I have hit this problem not just with Flycheck but also with using Scala 3 in Org Babel. I looked for a color option, but couldn't find it. Maybe it isn't documented? Maybe I'm bad at finding things. It wouldn't be the first time.

cayhorstmann commented 3 years ago

I don't know if this is the right place for this comment, but here goes: Indentation with quiet syntax is currently unusable.

Enter. Try typing
```
def fun = 
println("Hello") 
println("World") // Hitting Enter here does the wrong thing
```
Hitting Enter where indicated results in
```
def fun = 
println("Hello") 
println("World") // Hitting Enter here does the wrong thing
```
Sure, that's correctly formatted, but not the user's intent. The first rule ought to be "do no harm". If a user indented a line and then hits Enter, don't mess with the preceding line. The next line should have the same indent or one more (after def, if, class, etc.)
Tab.
```
def fun = 
println("Hello") 
println("World") // Hitting Tab here does the wrong thing
```
The Tab key should cycle through the valid indentations, like the Python mode does. It should be easy enough to find the number of valid indentations--either the same as the preceding line, or one more.

hvesalai commented 3 years ago

@cayhorstmann is this comment in relation to the WIP scala3 changes or also applicable to the scala2 mode?

cayhorstmann commented 3 years ago

In Scala 2, both Enter and Tab work as expected. I just checked with a Scala 2 project. The issue is the quiet (brace-free) syntax, which changes how Tab must work.

Kazark commented 3 years ago

@cayhorstmann yeah, this work is severely incomplete, and was interrupted for some time while I did not have access to a second monitor. I hope to pick it up again soon, but no promises on when I can get it finished; I have found it challenging, and we are not moving to 3 as rapidly as I had expected at work, so the push isn't as hard as I expected.

Kazark commented 3 years ago

I don't know. This is rather overwhelming.

Kazark commented 3 years ago

How far back, in general, should I look in order to correct indentation? If I have object Foo: at the very top of the file, and then one thousand lines that are not indented, and I try to fix the indentation of the thousandth line, do I look back one thousand lines?

Kazark commented 3 years ago

I suppose this situation is not fundamentally different than the situation with object { } previously. But previously, if you hit val or def, you knew it could not be top-level.

Kazark commented 3 years ago

Because my work on this has been so scattered, I forget a lot every time I go back to pick it up again. 😞

cayhorstmann commented 3 years ago

You should only look at the preceding line. If that's not correctly indented, it's not for you to fix. If the preceding line is one of the few special constructs that introduce a new indentation level (class, object, enum, then, else, while, for, try, etc.), then hitting Enter indents one more level, and Tab has one more level to cycle through. Otherwise Enter indents the same as the predecessor, and Tab cycles through the indents up to that level.

Kazark commented 3 years ago

@cayhorstmann thanks for the input

Kazark commented 2 years ago

c8a7bba may be the first passable draft here. Bleeding-edge folks feel free to give it a try and report your problems.

Kazark commented 2 years ago

I did find one regression myself already. Will try to get to it soon, but it is not serious compared to how badly this stuff was working up to this point.

rossabaker commented 2 years ago

Glad to see this moving along!

On c8a7bba, I can type with the expected indent after { and outdent after }:

abstract class Foo {
  def bar = 42
}

If the point is at or after the =, hitting tab leaves the line correctly indented. If the point is before the =, the line is outdented back to column 0.

This happens with compound keywords (e.g., abstract class, final class, sealed trait, case object), but not single keywords (e.g., class, trait, object).

Kazark commented 2 years ago

@rossabaker thank you for the report! I've fixed something akin to that before, but must not have covered that case. I'll see if I can get time to fix it today.

Kazark commented 2 years ago

Hm, okay, I think the similarity to the thing I already fixed is superficial. I'll have to think this one through.

jackcviers commented 2 years ago

First @Kazark - Thank you for working on this!

I just tried it and it doesn't seem to indent correctly with significant whitespace. Am I missing some additional configuration? See the video below:

Significant whitespace indent in scala 3

How to reproduce

Load this branch (I cloned into site-lisp and loaded it with the following elisp):

(add-to-list 'load-path "~/.emacs.d/site-lisp/emacs-scala-mode")
(require 'scala-mode)
(add-hook 'scala-mode-hook (lambda ()
             (lsp-mode)
             (setq prettify-symbols-alist scala-prettify-symbols-alist)
             (prettify-symbols-mode)
             (yas-reload-all)
             ))

Create a new scala-3 project:
```
$ sbt new scala/scala3.g8
```
Open the Main.scala file.
Attempt to edit without brackets.

Expected results:

Newline and indent after a definition should indent correctly under def.
Newline and indent after a correctly indented expression under a def should indent to the level of the previous expression.
Tab should cycle indentation depths.
indent-region should fix indentation.

Let me know how I can help!

Kazark commented 2 years ago

@jackcviers is "~/.emacs.d/site-lisp/emacs-scala-mode" a copy of this branch?

jackcviers commented 2 years ago

@jackcviers is "~/.emacs.d/site-lisp/emacs-scala-mode" a copy of this branch?

Git: Remote origin (git@github.com:Kazark/emacs-scala-mode.git) (2) @ master origin/master Merge pull request #165 from fommil/sbtn

OH! I think I see the issue. Should be on scala3 Extract function: scala-indent:continue-lookback?

@Kazark?

rossabaker commented 2 years ago

@jackcviers Yes, you need to be on the scala3 branch of his repo.

@Kazark 1ef3ce5 fixes the issue I reported on c8a7bba.

jackcviers commented 2 years ago

Ok. It's better! Nice work!

But still requires some manual tweaking -- tabs don't cycle indent levels:

Newline and indent doesn't cycle back on tab

Kazark commented 2 years ago

@jackcviers this is super WIP. Only a couple days ago did I even hit the first alpha version.

Kazark commented 2 years ago

@jackcviers TBH I've never used tabs to cycle indent levels in code, only in Org. I haven't looked into what makes that work, so I haven't even begun to try to get that working 😬

jackcviers commented 2 years ago

I realize that it's really new.

For indentation cycling, you need to see if the indent-to column output by the indent strategy is greater than the current indentation level. If it is greater, then you need to execute the indent. If it is equal to the current indentation level, then you indent-to column 0 instead. This shouldn't break your current setup (if I'm reading the code correctly). (and it doesn't!)

(defun scala-indent:indent-line (&optional strategy)
  "Indents the current line."
  (interactive "*")
  (let ((state (save-excursion (syntax-ppss (line-beginning-position)))))
    (cond ((< 0 (current-indentation))
       (scala-indent:indent-line-to (max 0 (- 0 (current-indentation)))))
      (
       (if (nth 8 state) ;; 8 = start pos of comment or string
               (scala-indent:indent-line-to
        (cond ((integerp (nth 4 state))    ;; 4 = nesting level of multi-line comment
                       (scala-indent:scaladoc-indent (nth 8 state)))
              ((eq t (nth 3 state))   ;; 3 = t for multi-line string
                       (or (save-excursion
                 (beginning-of-line)
                 (when (and (looking-at "\\s *|")
                    (progn (goto-char (nth 8 state))
                                               (looking-at "\\(\"\"\"\\)|")))
                               (goto-char (match-end 1))
                               (current-column)))
               (current-indentation)))
              (t (current-indentation))))
         (scala-indent:indent-code-line strategy)))
      )
    )
  )

Well, it almost works. Things that can be on their own won't re-indent now (like defs and vals, etc. I think I need to provide an extra variable to save the current-indent in, and let a third reset the indent to what it was previously. Let me noodle on it some more tomorrow, and I will have a cycle-indent function like what python-mode has that is a minimal intrusion to what you currently have in place (basically what I have here but with the default outside of a condition). I'll even feature flag it for you.

jackcviers commented 2 years ago

@Kazark -- https://github.com/Kazark/emacs-scala-mode/pull/1 FOR CONCEPTUAL REVIEW ONLY

I haven't tested it yet. Trying to figure out a way to test this with ERT before I try it in my editor. Expect more commits there. We can take comments over onto the PR. Let me know what you think of the idea.

Kazark commented 2 years ago

We should probably ask @hvesalai for a conceptual review of that as well

CLAassistant commented 2 years ago

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 4 committers have signed the CLA.

:white_check_mark: Kazark
:white_check_mark: jackcviers
:x: Jack Viers
:x: maemre

Jack Viers seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Kazark commented 2 years ago

@jackcviers ☝️ No rush, but want to make sure you see the CLA message above.

jackcviers commented 2 years ago

I see it! I'll get to it tonight after work.

Jack Viers - 515.313.7838 @.***

On Wed, May 18, 2022, 11:02 AM Keith Pinson @.***> wrote:

@jackcviers https://github.com/jackcviers ☝️ No rush, but want to make sure you see the CLA message above.

— Reply to this email directly, view it on GitHub https://github.com/hvesalai/emacs-scala-mode/pull/170#issuecomment-1130205318, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFBHFGWM3MK2PXAUI2CT4DVKUH7XANCNFSM42I5RXGA . You are receiving this because you were mentioned.Message ID: @.***>

jackcviers commented 2 years ago

@Kazark CLA signed, sealed and delivered. And you have a pr to your branch that fixes <TAB> and the @jj1uzh error discovered here.

jackcviers commented 2 years ago

They are all the same committed. I'll see if I can find the commit without an email address....

jackcviers commented 2 years ago

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.2 out of 3 committers have signed the CLA.✅ Kazark✅ jackcviers❌ Jack Viers Jack Viers seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it.

PR fixing missing email address in commit: https://github.com/Kazark/emacs-scala-mode/pull/3

VitalyAnkh commented 2 years ago

Oh why is this still blocked...

Kazark commented 2 years ago

@VitalyAnkh All the codebases I worked on (except breakable toys) are stuck on Scala 2 until, at a minimum, this gets fixed. As such, my motivation to work on this has stalled. If any wants to contribute, I'm happy to rebase the branch to get rid of the committer problem above, and open the branch for others to edit. You can also use what I've got so far (which is okay, but imperfect) but just downloading this branch and manually loading this version of the package.

maemre commented 1 year ago

I'd be happy to pick up the remaining work on this. I used Emacs Lisp only for my personal configuration before, but I have some free time to spend on this during the next few weeks.

Kazark commented 1 year ago

@maemre would you want to have a video call to hand stuff off? I don't know whether you will find my approach intelligible or not just reading the code.

maemre commented 1 year ago

@Kazark That'd be great! I can go through the code over the weekend and we can have a quick chat next week, if that sounds good. For scheduling the chat, my email address is maemre2 at gmail dot com

maemre commented 1 year ago

I edited my last comment because my email address wasn't formatted properly.

Kazark commented 1 year ago

Noted. I have an exceedingly busy week ahead---I'll hope to drop you a line next week.

VitalyAnkh commented 1 year ago

Hi! Just a kind reminder: how are things going now?

maemre commented 1 year ago

@VitalyAnkh thanks for the reminder, this completely fell through the cracks because I haven't been using Scala for the past few months. @Kazark would you be free sometime this week or next week to meet up? I am looking at the code base again, and some functions like scala-indent:analyze-syntax-stack are took a while to wrap my head around. The rest of scala-mode-indent.el makes sense though.

Also, from what I understand pretty much everything is implemented behind the custom variable scala-indent:use-cycle-indent already. And, the main issue with merging this upstream is the commit email addresses and the CLA for one specific commit. Is that correct?

I can try some git cherry-picking and re-implement the cycling feature that commit describes, then I think this should be good for a merge. @Kazark how does that sound?

Kazark commented 1 year ago

the main issue with merging this upstream is the commit email addresses and the CLA for one specific commit. Is that correct?

No, that part should be relatively easy to fix, I think. The main issue with merging is that this is still in a highly experimental state. I have been steadily using this branch locally for as long as it has existed, but I haven't written any Scala 3 beyond toys.

Kazark commented 1 year ago

The last time I had my head really into this, there were definitely some cases that didn't work right. Ultimately it is up to @hvesalai ; we could just plunge in & patch; but from my point of view, this is not stable yet.

hvesalai / emacs-scala-mode

WIP: Scala 3 #170

How to reproduce

Expected results: