[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.