Discussion:
[erlang-patches] [erlang-bugs] erlang.el indenter
unknown
2012-08-21 21:00:12 UTC
Permalink
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,

Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.

git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string

https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch

Here's an illustration of what the patch does,
excerpt from the commit msg:

Previously, typing C-M-\ or M-x indent-region RET on the following:

somefunction() ->
S = <<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).

resulted in this (note the "text2" line):

somefunction() ->
S = <<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).

Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.


BRs
Tomas
unknown
2012-08-22 10:14:17 UTC
Permalink
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!

BR Fredrik G
Erlang/OTP Team
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
BRs
Tomas
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches
unknown
2012-08-22 17:10:10 UTC
Permalink
Post by unknown
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
Post by unknown
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
BRs
Tomas
unknown
2012-08-22 21:42:07 UTC
Permalink
Post by unknown
Post by unknown
Here is a patch for indenting of multi-line strings
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
I see what you mean. Will try to fix it.

BRs
Tomas
unknown
2012-08-22 22:07:02 UTC
Permalink
Post by unknown
Post by unknown
Post by unknown
Here is a patch for indenting of multi-line strings
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
I see what you mean. Will try to fix it.
Thanks. Are the erlang.el changes in 'pu' susceptible to new
mis-indentations by nature? The original issue is not important
enough to introduce regressions if fixing it means erlang.el
will get imprecise or hard to get right.
Out of curiosity, what about using erl_parse, erl_scan, etc. via an
escript instead? Does that sound viable and make sense?
unknown
2012-08-28 21:10:25 UTC
Permalink
Post by unknown
Post by unknown
Post by unknown
Post by unknown
Here is a patch for indenting of multi-line strings
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
I see what you mean. Will try to fix it.
Thanks. Are the erlang.el changes in 'pu' susceptible to new
mis-indentations by nature? The original issue is not important
enough to introduce regressions if fixing it means erlang.el
will get imprecise or hard to get right.
For the time being, would it be possible to
back this patch out from pu? Not sure if I can fix it
in time for the upcoming release.

Apologies for any trouble this may cause.

The solution path I ventured into was not a good one,
I realized. I had only tried it with an older version
of rebar, so didn't catch the mis-indentation.
Post by unknown
Out of curiosity, what about using erl_parse, erl_scan, etc. via an
escript instead? Does that sound viable and make sense?
That might be a possibility, although, it might
require another grammar than the ordinary erlang
grammar, there are some issues one would need
to handle: being able to indent before the function
is completely written, and being able to indent
comments. Also, I guess macros could be a
challenge.

BRs
Tomas
unknown
2012-08-29 14:23:18 UTC
Permalink
Post by unknown
Post by unknown
Post by unknown
Post by unknown
Post by unknown
Here is a patch for indenting of multi-line strings
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
I see what you mean. Will try to fix it.
Thanks. Are the erlang.el changes in 'pu' susceptible to new
mis-indentations by nature? The original issue is not important
enough to introduce regressions if fixing it means erlang.el
will get imprecise or hard to get right.
For the time being, would it be possible to
back this patch out from pu? Not sure if I can fix it
in time for the upcoming release.
That's no problem, just get back to me when you have a new version.
Post by unknown
Apologies for any trouble this may cause.
The solution path I ventured into was not a good one,
I realized. I had only tried it with an older version
of rebar, so didn't catch the mis-indentation.
Post by unknown
Out of curiosity, what about using erl_parse, erl_scan, etc. via an
escript instead? Does that sound viable and make sense?
That might be a possibility, although, it might
require another grammar than the ordinary erlang
grammar, there are some issues one would need
to handle: being able to indent before the function
is completely written, and being able to indent
comments. Also, I guess macros could be a
challenge.
BRs
Tomas
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches
--
/Henrik Nord Erlang/OTP
unknown
2014-01-24 14:55:07 UTC
Permalink
Ping?!

Any update regarding this?
Post by unknown
Post by unknown
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
Post by unknown
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
BRs
Tomas
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches
--
/Henrik Nord Erlang/OTP
unknown
2014-01-24 16:49:53 UTC
Permalink
Post by unknown
Ping?!
Any update regarding this?
Fredrik's patch introduced a regression, so should not be merged, but
the bugs in the indenter are still there.

Steve Vinoski also analyzed the issues a couple days ago, and he has
a more complete understanding of the Elisp code. Steve, can you
summarize what's broken?
Post by unknown
Post by unknown
Post by unknown
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
Post by unknown
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
unknown
2014-01-24 16:52:56 UTC
Permalink
Post by unknown
Post by unknown
Ping?!
Any update regarding this?
Fredrik's patch introduced a regression, so should not be merged, but
Sorry, that should have said Thomas (not Fredrik).
Post by unknown
the bugs in the indenter are still there.
Steve Vinoski also analyzed the issues a couple days ago, and he has
a more complete understanding of the Elisp code. Steve, can you
summarize what's broken?
Post by unknown
Post by unknown
Post by unknown
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
Post by unknown
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
unknown
2014-01-24 20:29:48 UTC
Permalink
Post by unknown
Post by unknown
Post by unknown
Ping?!
Any update regarding this?
Fredrik's patch introduced a regression, so should not be merged, but
Sorry, that should have said Thomas (not Fredrik).
Hehe, that should have said Tomas (not Thomas) :)

Anyway, I agree this patch shouldn't be included. Thanks
for noticing. I do not have any improvement to it, and don't
think I will have for the near future . Best woul be to just
forget about the patch. Or even better would of course
if someone (Steve?) makes a better patch

BRs
Tomas
Post by unknown
Post by unknown
the bugs in the indenter are still there.
Steve Vinoski also analyzed the issues a couple days ago, and he has
a more complete understanding of the Elisp code. Steve, can you
summarize what's broken?
Post by unknown
Post by unknown
Post by unknown
Hello Tomas,
We have included this patch in the 'pu' branch.
Thank you for the contribution!
A quick test revealed that erlang.el from 'pu' erroneously
re-indents the already correctly indented rebar:option_spec_list/0
and also rebar:commands/0. Can you reproduce the regression?
Post by unknown
Post by unknown
There seems to be a bug in the indenter that mis-indents
rebar.erl:commands/0, if it's indented as part of a larger region.
The multi-line binary string seems to be the problem.
https://github.com/basho/rebar/blob/491d52298e2/src/rebar.erl#L275-311
It works correctly when indenting the function itself with C-c C-q.
Hi,
Here is a patch for indenting of multi-line strings, like the
one previously reported by Tuncer Ayaz.
git fetch git://github.com/tomas-abrahamsson/otp.git
emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string
https://github.com/tomas-abrahamsson/otp/compare/emacs-indent-multi-line-string.patch
Here's an illustration of what the patch does,
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
somefunction() ->
S =<<"
text1 somemore1
text2 somemore2
">>,
io:put_chars(S).
Now, the indentation inside the multi-line string is left unchanged.
This is in line with how the Emacs C and Lisp mode treats multi-line
strings when indenting regions.
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches
unknown
2014-01-26 23:25:31 UTC
Permalink
Post by unknown
Post by unknown
Ping?!
Any update regarding this?
Fredrik's patch introduced a regression, so should not be merged, but
the bugs in the indenter are still there.
Steve Vinoski also analyzed the issues a couple days ago, and he has
a more complete understanding of the Elisp code. Steve, can you
summarize what's broken?
In August 2013 Magnus Henoch posted a suggestion to use emacs syntax
properties to solve these kinds of issues:

http://erlang.org/pipermail/erlang-bugs/2013-August/003729.html

I agree that's the right way to go, but I don't think it's a small amount
of work.

--steve
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20140126/97e71234/attachment.html>
unknown
2014-02-03 09:09:43 UTC
Permalink
Ok!

I will consider this particular patch closed.

If anyone does find the time to implement the other suggestion they will
have to submit a new patch

Thank you for your contributions!
On Fri, Jan 24, 2014 at 11:49 AM, Tuncer Ayaz <tuncer.ayaz
On Fri, Jan 24, 2014 at 3:55 PM, Henrik Nord <henrik
Post by unknown
Ping?!
Any update regarding this?
Fredrik's patch introduced a regression, so should not be merged, but
the bugs in the indenter are still there.
Steve Vinoski also analyzed the issues a couple days ago, and he has
a more complete understanding of the Elisp code. Steve, can you
summarize what's broken?
In August 2013 Magnus Henoch posted a suggestion to use emacs syntax
http://erlang.org/pipermail/erlang-bugs/2013-August/003729.html
I agree that's the right way to go, but I don't think it's a small
amount of work.
--steve
--
/Henrik Nord Erlang/OTP

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20140203/3aeac354/attachment.html>
unknown
2014-02-03 13:10:24 UTC
Permalink
As much as I hate GitHub, that sounds like a good use case for its Issues system.
--
Anthony Ramine
If anyone does find the time to implement the other suggestion they will have to submit a new patch
Loading...