search

emacs-ess / ESS

Emacs Speaks Statistics: ESS
https://ess.r-project.org/
GNU General Public License v3.0
626 stars 161 forks source link

broken indentation after continued string #379

Open thisirs opened 8 years ago

thisirs commented 8 years ago

Hi all,

I have the following behaviour with latest melpa ESS with a discontinued indentation with continued strings ;)

blah(first,
     "continued
string",
broken)

Is there an option controlling this or is it a bug?

lionel- commented 8 years ago

Hi, this is standard Emacs behaviour within strings.

lionel- commented 8 years ago

Note that if it was indented you would have spaces inside your string which in many case is not what's expected. So strings should not be indented except with languages featuring string dedentation like Julia.

thisirs commented 8 years ago

I don't want my string to be indented. I want broken to be indented just like first and the string!

lionel- commented 8 years ago

ah yes, the rule is to never indent past leftmost indentation.

thisirs commented 8 years ago

What's even more odd is that we have:

blah(first,
     "continued
            string",
     broken)

So the indentation of broken depends on what the string contains...

lionel- commented 8 years ago

yes that's because of the rule I mentioned, otherwise we'd have ugly hanging indents in those cases:

lapply(x, function(y) {
  NULL
},
       arg)
vspinu commented 8 years ago

@lionel- , but aren't we supposed to treat strings as a literals and indent with respect to the beginnings of those instead of the most recent lines?

lionel- commented 8 years ago

I'm not sure I get what you're saying @vspinu

vspinu commented 8 years ago

I am saying that 

blah(first,
     "continued
string",
broken)

should be indented as 


blah(first,
     "continued
string",
     broken)

That is broken should be indented with respect to the beginning of the whole string"continued and not aligned to string".

lionel- commented 8 years ago

ok I thought you were suggesting something else than OP. I think it makes sense it's just that it's very low priority stuff. Would you like to send a PR @thisirs ?

thisirs commented 8 years ago

@lionel- I can give it a try. Some pointers are welcome !

lionel- commented 8 years ago

The relevant function is ess-maximum-args-indent in ess-r-d.el. It currently checks all arguments but it really should only check the last one (but that can stay like this for now).

lionel- commented 8 years ago

Run make indent-cases in the test folder and check if any of the test file has changed (i.e. has broken).