unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
       [not found] <E1VeoWf-0002wa-2m@vcs.savannah.gnu.org>
@ 2013-11-08 17:58 ` Dmitry Gutov
  2013-11-08 18:23   ` Bozhidar Batsov
  2013-11-08 19:04   ` Stefan Monnier
  0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Gutov @ 2013-11-08 17:58 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: emacs-devel

Bozhidar Batsov <bozhidar@batsov.com> writes:

> ------------------------------------------------------------
> revno: 115036
> revision-id: bozhidar@batsov.com-20131108160155-qqqzvzyv7buc7r1v
> parent: dgutov@yandex.ru-20131108112252-g8bofo4jray5k45v
> committer: Bozhidar Batsov <bozhidar@batsov.com>
> branch nick: master
> timestamp: Fri 2013-11-08 18:01:55 +0200

> +(defcustom ruby-encoding-magic-comment-style 'ruby

I though we agreed that the default won't change: still Emacs.  AFAIK,
it has the benefit of making sure Emacs uses the specified encoding.

I could be wrong, though.

> +  "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?

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
  2013-11-08 17:58 ` trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use Dmitry Gutov
@ 2013-11-08 18:23   ` Bozhidar Batsov
  2013-11-08 19:16     ` Dmitry Gutov
  2013-11-08 19:04   ` Stefan Monnier
  1 sibling, 1 reply; 6+ messages in thread
From: Bozhidar Batsov @ 2013-11-08 18:23 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]

On 8 November 2013 19:58, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Bozhidar Batsov <bozhidar@batsov.com> writes:
>
> > ------------------------------------------------------------
> > revno: 115036
> > revision-id: bozhidar@batsov.com-20131108160155-qqqzvzyv7buc7r1v
> > parent: dgutov@yandex.ru-20131108112252-g8bofo4jray5k45v
> > committer: Bozhidar Batsov <bozhidar@batsov.com>
> > branch nick: master
> > timestamp: Fri 2013-11-08 18:01:55 +0200
>
> > +(defcustom ruby-encoding-magic-comment-style 'ruby
>
> I though we agreed that the default won't change: still Emacs.  AFAIK,
> it has the benefit of making sure Emacs uses the specified encoding.
>
> I could be wrong, though.
>

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've never had problems with Emacs 24 using only Ruby-style
comments. I might be missing something, though.


>
> > +  "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.


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

[-- Attachment #2: Type: text/html, Size: 3452 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
  2013-11-08 17:58 ` trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use Dmitry Gutov
  2013-11-08 18:23   ` Bozhidar Batsov
@ 2013-11-08 19:04   ` Stefan Monnier
  2013-11-08 19:48     ` Bozhidar Batsov
  1 sibling, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2013-11-08 19:04 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Bozhidar Batsov, emacs-devel

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

This will use the first branch if ruby-encoding-magic-comment-style is
`quote' (and the two other branches will also check to see if
ruby-encoding-magic-comment-style is equal to `quote' tho that check
will be redundant).


        Stefan



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
  2013-11-08 18:23   ` Bozhidar Batsov
@ 2013-11-08 19:16     ` Dmitry Gutov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Gutov @ 2013-11-08 19:16 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: emacs-devel

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.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
  2013-11-08 19:04   ` Stefan Monnier
@ 2013-11-08 19:48     ` Bozhidar Batsov
  2013-11-08 20:35       ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Bozhidar Batsov @ 2013-11-08 19:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Dmitry Gutov

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

So you're saying I should use `cond', right? I was under the impression
that `case' is preferable if you'll be always checking the value of the
same expression.


On 8 November 2013 21:04, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> >> +                            (case ruby-encoding-magic-comment-style
> >> +                              ('ruby "# coding: %s")
> >> +                              ('emacs "# -*- coding: %s -*-")
> >> +                              ('custom
> ruby-custom-encoding-magic-comment-template))))
>
> This will use the first branch if ruby-encoding-magic-comment-style is
> `quote' (and the two other branches will also check to see if
> ruby-encoding-magic-comment-style is equal to `quote' tho that check
> will be redundant).
>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 1353 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use
  2013-11-08 19:48     ` Bozhidar Batsov
@ 2013-11-08 20:35       ` Stefan Monnier
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2013-11-08 20:35 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: emacs-devel, Dmitry Gutov

> So you're saying I should use `cond', right? I was under the impression
> that `case' is preferable if you'll be always checking the value of the
> same expression.

No, I'm saying that the format of a "case" branch is (CASES &rest
FORMS), where CASES can be a list of values.  In your case, CASES is
"'ruby" which is a shorthand for "(quote ruby)", i.e. you test 2 values.


        Stefan



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-11-08 20:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <E1VeoWf-0002wa-2m@vcs.savannah.gnu.org>
2013-11-08 17:58 ` trunk r115036: * lisp/progmodes/ruby-mode.el (ruby-mode-set-encoding): Use Dmitry Gutov
2013-11-08 18:23   ` Bozhidar Batsov
2013-11-08 19:16     ` Dmitry Gutov
2013-11-08 19:04   ` Stefan Monnier
2013-11-08 19:48     ` Bozhidar Batsov
2013-11-08 20:35       ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).