search

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

feat: support trailing commas (>= Scala 2.12.2) #167

Closed Kazark closed 3 years ago

Kazark commented 3 years ago

Trailing commas were added in Scala 2.12.2.

Thought that before I jumped all the way in on the Scala 3 upgrade I would try patching something that has been bugging me w.r.t. the Scala 2 implementation.

Not 100% sure this is the most surgical fix. Will be testing locally and would love feedback.

hvesalai commented 3 years ago

Would this interfere with somebody using auto pairs for parenthesis? If so, should it be made configurable

Kazark commented 3 years ago

Looks good. Did you test it with different kind of test cases?

What I was going to do was just use it in my coding for several days and see if I noticed any anomalies. However I'm happy to do some more targeted testing as well. Suggestions on test cases welcome, but I can cook up some of my own.

Would this interfere with somebody using auto pairs for parenthesis?

Sorry to be slow here---what are auto pairs? Is that where when you type (, the ) also appears?

hvesalai commented 3 years ago

What I was going to do was just use it in my coding for several days and see if I noticed any anomalies. However I'm happy to do some more targeted testing as well. Suggestions on test cases welcome, but I can cook up some of my own.

You could try some coding conventions that you don't use your self. See the README.md

Sorry to be slow here---what are auto pairs? Is that where when you type (, the ) also appears?

Yes, exactly

Kazark commented 3 years ago

Will do some more targeted testing and report back; I will try varying coding conventions. I'm going to be doing some travel this week so it might not be snappy.

I do have auto pair on---I guess it's on by default in Spacemacs. I will test without as well.

Kazark commented 3 years ago

@hvesalai alright, I've tried every combination I could think of for this, including testing without smart parens. It looks good to me.

hvesalai commented 3 years ago

Ok, let's merge it then!