[erlang-patches] [PATCH] Fix indentation of record fields in Emacs

[Tomas Abrahamsson]

Hi,

I never received any feedback on the patch to the emacs
record indentation, it was not included in pu and not the R15B01.
Was it good? Bad? Ugly? Or just forgotten?

We've been using it extensively in a project with lots
of records, and folks seem happy with it. We haven't
seen any bad cases of indentation due to it.

BRs
Tomas

On Sat, Feb 11, 2012 at 9:55 PM, Tomas Abrahamsson
<tomas.abrahamsson@REDACTED> wrote:
> In some situations, the indentation of record fields in Emacs was
> strange. This example below shows how Emacs previously would indent
> two similar pieces of code very differently:
>
>    some_function_with_a_very_long_name() ->
>        #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
>                                         field1=a,
>                                         field2=b}.
>
>    x() ->
>        #some_record_name{
>       field1=a,
>       field2=b}.
>
> This changes the indentation to be like below for both cases:
>
>    some_function() ->
>        #some_record{
>           field1=a,
>           field2=b}.
> ---
>  lib/tools/emacs/erlang.el         |   58 +++++++++++++++++++++++++++++-------
>  lib/tools/emacs/test.erl.indented |   38 ++++++++++++++++++++++++
>  lib/tools/emacs/test.erl.orig     |   38 ++++++++++++++++++++++++
>  3 files changed, 122 insertions(+), 12 deletions(-)
>
> diff --git a/lib/tools/emacs/erlang.el b/lib/tools/emacs/erlang.el
> index bc7a190..2f6c7f5 100644
> --- a/lib/tools/emacs/erlang.el
> +++ b/lib/tools/emacs/erlang.el
> @@ -2986,18 +2986,52 @@ This assumes that the preceding expression is either simple
>     (forward-sexp (- arg))
>     (let ((col (current-column)))
>       (skip-chars-backward " \t")
> -      ;; Needed to match the colon in "'foo':'bar'".
> -      (if (not (memq (preceding-char) '(?# ?:)))
> -          col
> -        ;; Special hack to handle: (note line break)
> -        ;; [#myrecord{
> -        ;;  foo = foo}]
> -        (or
> -         (ignore-errors
> -           (backward-char 1)
> -           (forward-sexp -1)
> -           (current-column))
> -         col)))))
> +      ;; Special hack to handle: (note line break)
> +      ;; [#myrecord{
> +      ;;  foo = foo}]
> +      ;; where the call (forward-sexp -1) will fail when point is at the `#'.
> +      (or
> +       (ignore-errors
> +        ;; Needed to match the colon in "'foo':'bar'".
> +        (cond ((eq (preceding-char) ?:)
> +               (backward-char 1)
> +               (forward-sexp -1)
> +               (current-column))
> +              ((eq  (preceding-char) ?#)
> +               ;; We may now be at:
> +               ;; - either a construction of a new record
> +               ;; - or update of a record, in which case we want
> +               ;;   the column of the expression to be updated.
> +               ;;
> +               ;; To see which of the two cases we are at, we first
> +               ;; move an expression backwards, check for keywords,
> +               ;; then immediately an expression forwards.  Moving
> +               ;; backwards skips past tokens like `,' or `->', but
> +               ;; when moving forwards again, we won't skip past such
> +               ;; tokens.  We use this: if, after having moved
> +               ;; forwards, we're back where we started, then it was
> +               ;; a record update.
> +               ;; The check for keywords is to detect cases like:
> +               ;;   case Something of #record_construction{...}
> +               (backward-char 1)
> +               (let ((record-start (point))
> +                     (record-start-col (current-column)))
> +                 (forward-sexp -1)
> +                 (let ((preceding-expr-col (current-column))
> +                       ;; white space definition according to erl_scan
> +                       (white-space "\000-\040\200-\240"))
> +                   (if (erlang-at-keyword)
> +                       ;; The (forward-sexp -1) call moved past a keyword
> +                       (1+ record-start-col)
> +                     (forward-sexp 1)
> +                     (skip-chars-forward white-space record-start)
> +                     ;; Are we back where we started?  If so, it was an update.
> +                     (if (= (point) record-start)
> +                           preceding-expr-col
> +                       (goto-char record-start)
> +                       (1+ (current-column)))))))
> +              (t col)))
> +       col))))
>
>  (defun erlang-indent-parenthesis (stack-position)
>   (let ((previous (erlang-indent-find-preceding-expr)))
> diff --git a/lib/tools/emacs/test.erl.indented b/lib/tools/emacs/test.erl.indented
> index 2948ccf..e0593c6 100644
> --- a/lib/tools/emacs/test.erl.indented
> +++ b/lib/tools/emacs/test.erl.indented
> @@ -657,3 +657,41 @@ indent_comprehensions() ->
>  foo() ->
>     [#foo{
>        foo = foo}].
> +
> +%% Record indentation
> +some_function_with_a_very_long_name() ->
> +    #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> +       field1=a,
> +       field2=b},
> +    case dummy_function_with_a_very_very_long_name(x) of
> +       #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> +          field1=a,
> +          field2=b} ->
> +           ok;
> +       Var = #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> +                field1=a,
> +                field2=b} ->
> +           Var#'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> +             field1=a,
> +             field2=b};
> +       #xyz{
> +          a=1,
> +          b=2} ->
> +           ok
> +    end.
> +
> +another_function_with_a_very_very_long_name() ->
> +    #rec{
> +       field1=1,
> +       field2=1}.
> +
> +some_function_name_xyz(xyzzy, #some_record{
> +                                field1=Field1,
> +                                field2=Field2}) ->
> +    SomeVariable = f(#'Some-long-record-name'{
> +                       field_a = 1,
> +                       'inter-xyz-parameters' =
> +                           #'Some-other-very-long-record-name'{
> +                              field2 = Field1,
> +                              field2 = Field2}}),
> +    {ok, SomeVariable}.
> diff --git a/lib/tools/emacs/test.erl.orig b/lib/tools/emacs/test.erl.orig
> index 1221c56..69356ac 100644
> --- a/lib/tools/emacs/test.erl.orig
> +++ b/lib/tools/emacs/test.erl.orig
> @@ -657,3 +657,41 @@ ok.
>  foo() ->
>  [#foo{
>  foo = foo}].
> +
> +%% Record indentation
> +some_function_with_a_very_long_name() ->
> + #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> + field1=a,
> + field2=b},
> + case dummy_function_with_a_very_very_long_name(x) of
> + #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> + field1=a,
> + field2=b} ->
> + ok;
> + Var = #'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> + field1=a,
> + field2=b} ->
> + Var#'a-long-record-name-like-it-sometimes-is-with-asn.1-records'{
> + field1=a,
> + field2=b};
> + #xyz{
> + a=1,
> + b=2} ->
> + ok
> + end.
> +
> +another_function_with_a_very_very_long_name() ->
> + #rec{
> + field1=1,
> + field2=1}.
> +
> +some_function_name_xyz(xyzzy, #some_record{
> + field1=Field1,
> + field2=Field2}) ->
> + SomeVariable = f(#'Some-long-record-name'{
> + field_a = 1,
> + 'inter-xyz-parameters' =
> + #'Some-other-very-long-record-name'{
> + field2 = Field1,
> + field2 = Field2}}),
> + {ok, SomeVariable}.
> --
> 1.7.5.4
>