Groovy-Emacs-Modes / groovy-emacs-modes

A groovy major mode, grails minor mode, and a groovy inferior mode.
84 stars 39 forks source link

Comment handling is broken #51

Closed russel closed 7 years ago

russel commented 7 years ago

The font-lock behaviour with comments is wrong. // without a following space is not treated as a comment. // with a following space is.

jgrzebyta commented 7 years ago

also /* is not treated as a comment.

Gentoo mode version: 20170516.1155 Emacs: 25.1.1

Attached screenshot:

gentoo_file

russel commented 7 years ago

I think we need @Wilfred to take a look at this if he can.

Wilfred commented 7 years ago

I cannot replicate either of the issues mentioned.

Given the text:

// foo

def x = 1

//bar

def y = 2

/* baz */

def z = 2

I'm seeing:

screen shot 2017-05-26 at 17 27 17

Please provide some example Groovy code that I can copy and paste into Emacs.

It's also possible that you have some state in your Emacs instance, so if you've just upgraded from the previous version of groovy-mode, I suggest restarting Emacs.

Wilfred commented 7 years ago

p.s. thanks for pinging me, I would have missed this otherwise.

russel commented 7 years ago

Try

// foo
//

def x = 1

//bar

def y = 2

/* baz */

def z = 2

where there is no space after the // following // foo. Then add a space and remove it again. Quite weird.

Wilfred commented 7 years ago

Thanks, I can reproduce. Looks like a bug in groovy-stringify-slashy-string which is treating the second / in // as an opening string delimiter. The comment logic isn't quite right.

andreas-marschke commented 7 years ago

Theres another issue related to that I think when defining inline Patterns in Groovy screen shot 2017-06-12 at 15 02 09

Example code:

package com.example.groovy

class ExampleClass {

    private ArrayList<Pattern> _patterns = new ArrayList<Pattern> ([
            ~/.*example-[0-9\.]*\.foo$/
    ])

    public ExampleClass(int num) {

    }
}

@Wilfred / @russel Could you test if this also happens in your revisions? Updated to most recent seems to still be the case.

vincekd commented 7 years ago

Also seeing this, as well as @andreas-marschke's addition.

vincekd commented 7 years ago

screenshot from 2017-06-28 18-10-19

Looks like there's a conflict between regular slash-quotes and dollar-sign slash quotes in the problem reported by @andreas-marschke.

I've never dealt with emacs syntax parsing, but I'll look at it tomorrow.

grdryn commented 7 years ago

I'm not sure if this is related or not (it sounds like it could be): it appears that a single-line comment ending with a question mark will lead to the next line being indented an extra level. E.g.:

// Could this be better?
    def x = 1

rather than

// Could this be better?
def x = 1

Using a multi-line quote instead works fine:

/* Could this be better? */
def x = 1
Wilfred commented 7 years ago

@grdryn it's not related: this is highlighting, whereas that's indentation.

However, that's a definite bug, which I've fixed in #72 :)

grdryn commented 7 years ago

@Wilfred that was quick...thanks a lot! :)

vincekd commented 7 years ago

Hi @Wilfred, after your fix a / for division opens a slashy-string.

Wilfred commented 7 years ago

Argh, groovy syntax is tricky. I've opened https://stackoverflow.com/questions/45234712/how-does-groovy-distinguish-division-from-strings but it's just not clear to me when Groovy thinks / is a slashy-string delimiter vs when / is divison.

vincekd commented 7 years ago

Seems tricky. At the very least we know that the slashy string should never go multiple lines (so if there is no termination on that line then it must be division). screenshot_20170721_101035

EDIT: I just saw that you said the slashy-string should be multi-line in your SO post. I thought that was the purpose of the $/ /$ slashy-string and that the regular slashy string was just single line?

Wilfred commented 7 years ago

Slashy-strings can be multiline: http://groovy-lang.org/syntax.html#_slashy_string

I think the correct thing to do is to look at the previous non-whitespace character. If it's ( or an operator like = or + it's a slashy-string. Otherwise, it's just division.

vincekd commented 7 years ago

I played around a bit and got it working, but I have no idea if it's the correct way to do it.

Commit: https://github.com/vincekd/groovy-emacs-modes/commit/d096d15f6d0f85ad94bd820d03de9e2d0650f6e0

screenshot_20170721_135246

Wilfred commented 7 years ago

I think all the issues raised here have been fixed:

Highlighting:

Indentation:

Please file separate issues for anything else that crops up.