emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding


From: Dmitry Gutov
Subject: Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
Date: Fri, 08 Nov 2013 21:16:33 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0

On 08.11.2013 20:23, Bozhidar Batsov wrote:
I had a last minute change of heart regarding this. Based on my
observations much more Ruby hackers use ruby-style encoding comments, so
I thought this makes more sense as a default. AFAIK the Emacs encoding
comments made a lot of difference in the pre-Emacs 24 era, but are not
that important now.

I don't really know the history of this feature, but it seems to be the most popular/useful for the Japanese users, and they're not very visible on emacs-devel or the bug tracker. Maybe it's nothing.

My stance is purely based on the long-standing (and often criticized, sure) Emacs tradition to not change the defaults without solid reasons.

     > +  "The style of the magic encoding comment to use."
     > +  :type '(choice
     > +          (const :tag "Emacs Style" emacs)
     > +          (const :tag "Ruby Style" ruby)
     > +          (const :tag "Custom Style" custom))
     > +  :group 'ruby)
     > +
     > +(defcustom ruby-custom-encoding-magic-comment-template "#
    coding: %s"

    The way this template is duplicated below for the `ruby' case doesn't
    look too nice.  Since you went ahead with symbol-based customization,
    why not drop the "custom" case?


I'm not sure what exactly the problem is with the current code. Someone
might want to use a different encoding format, so the custom option
seems reasonable to me. The default value of the custom template doesn't
really matter as users interested in using a custom format will surely
alter it.

My point is:

1) The `custom' option won't be used too often, if at all.
2) Even if that option stays, using whole two variables is an overkill, you can just replace the symbols inside the :type form with their corresponding template values and thus eliminate the `case' form below.

     > -                     (insert "# -*- coding: " coding-system "
    -*-\n"))))
     > +                     (let ((encoding-magic-comment-template
     > +                            (case ruby-encoding-magic-comment-style
     > +                              ('ruby "# coding: %s")
     > +                              ('emacs "# -*- coding: %s -*-")
     > +                              ('custom

These are rather minor issues, so I'm not going to argue them further.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]