Text Mode Menu

Text Mode Menu

[John Anthony]

Hello, everybody.

I'm preparing to bundle up my first patch (hopefully of many) to send
off to bug-gnu-emacs and I thought I'd ask for any style advice or
really any advice in general I can get before doing so in the hopes of
being successful. Here's the patch:


diff --git a/lisp/textmodes/text-mode.el b/lisp/textmodes/text-mode.el
index d9ff04c..d4e65a7 100644
--- a/lisp/textmodes/text-mode.el
+++ b/lisp/textmodes/text-mode.el
@@ -51,6 +51,26 @@ Use (derived-mode-p 'text-mode) instead.")
 (defvar text-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\e\t" 'ispell-complete-word)
+    (define-key map [menu-bar text]
+      (cons "Text" (make-sparse-keymap "Text")))
+    (bindings--define-key map [menu-bar text toggle-text-mode-auto-fill]
+      '(menu-item "Text Mode Auto Fill" toggle-text-mode-auto-fill
+                  :button (:toggle . (memq 'turn-on-auto-fill text-mode-hook))
+                  :help "Toggle auto fill within text modes"))
+    (bindings--define-key map [menu-bar text paragraph-indent-minor-mode]
+      '(menu-item "Paragraph Indent Minor Mode" paragraph-indent-minor-mode
+                  :button (:toggle . (bound-and-true-p paragraph-indent-minor-mode))
+                  :help "Toggle paragraph indent minor mode"))
+    (bindings--define-key map [menu-bar text center-region]
+      '(menu-item "Center Region" center-region
+                  :help "Center the marked region"
+                  :enable (region-active-p)))
+    (bindings--define-key map [menu-bar text center-paragraph]
+      '(menu-item "Center Paragraph" center-paragraph
+                  :help "Center the current paragraph"))
+    (bindings--define-key map [menu-bar text center-line]
+      '(menu-item "Center Line" center-line
+                  :help "Center the current line"))
     map)
   "Keymap for `text-mode'.
 Many other modes, such as `mail-mode', `outline-mode' and `indented-text-mode',


I aped the style of lisp-mode.el, so hopefully I won't have gone too
far wrong. Please do tell me if I'm about to do something totally
wrong and feel free to ignore me if I'm being unnecessarily
cautious. If I'm being so cautious that I'm wasting peoples' time by
asking about this here rather than just sending it to bug-gnu-emacs
then please do tell me.

Thanks in advance, everyone.

--
JA

RE: Text Mode Menu

[Drew Adams]

Two minor suggestions -

> +      '(menu-item "Text Mode Auto Fill" 

`Auto Fill'         - toggle button shows that it is modal

> +      '(menu-item "Paragraph Indent Minor Mode"

`Indent Paragraphs' - toggle button shows that it is modal

Re: Text Mode Menu

[John Anthony]

On Sat, Oct 05, 2013 at 07:24:58AM -0700, Drew Adams wrote:
> Two minor suggestions -
> 
> > +      '(menu-item "Text Mode Auto Fill" 
> 
> `Auto Fill'         - toggle button shows that it is modal
> 
> > +      '(menu-item "Paragraph Indent Minor Mode"
> 
> `Indent Paragraphs' - toggle button shows that it is modal

Could you clarify this at all? Do you just mean clearer labels or
could you give an example of a menu item that shows that the option is
modal?

---
J.A.

RE: Text Mode Menu

[Drew Adams]

> > Two minor suggestions -
> >
> > > +      '(menu-item "Text Mode Auto Fill"
> >
> > `Auto Fill'         - toggle button shows that it is modal
> >
> > > +      '(menu-item "Paragraph Indent Minor Mode"
> >
> > `Indent Paragraphs' - toggle button shows that it is modal
> 
> Could you clarify this at all? Do you just mean clearer labels or
> could you give an example of a menu item that shows that the option is
> modal?

Yes, suggestions for the labels.

Your code (which I did not try) indicates that you use toggle buttons: 
:button (:toggle . (memq 'turn-on-auto-fill text-mode-hook))

They should thus appear similar to what we see in the Options menu.
You can tell by looking at the menu item and its checkmark (or lack
thereof) that the item represents something with two states ("modes"),
and you can tell what the current state is.

This means that you need not add words in the menu item label to say
that the thing is modal: "... Minor Mode".  (Yes, one cannot tell from
the toggle button alone that the mode is minor, but that is only minor
info. ;-))

Re: Text Mode Menu

[Davis Herring]

> This means that you need not add words in the menu item label to say
> that the thing is modal: "... Minor Mode".  (Yes, one cannot tell from
> the toggle button alone that the mode is minor, but that is only minor
> info. ;-))

Sure you can.  You can't turn off a major mode (unless you count
`fundamental-mode').

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

RE: Text Mode Menu

[Drew Adams]

> > This means that you need not add words in the menu item label to say
> > that the thing is modal: "... Minor Mode".  (Yes, one cannot tell from
> > the toggle button alone that the mode is minor, but that is only minor
> > info. ;-))
> 
> Sure you can.  You can't turn off a major mode (unless you count
> `fundamental-mode').

Good point.  What you cannot tell from looking at a toggle button is
whether it represents a two-state variable or a minor mode.  But who
cares, in this context?

Re: Text Mode Menu

[John Anthony]

On Mon, Oct 07, 2013 at 10:22:21AM -0700, Drew Adams wrote:
> 
> > > Two minor suggestions -
> > >
> > > > +      '(menu-item "Text Mode Auto Fill"
> > >
> > > `Auto Fill'         - toggle button shows that it is modal
> > >
> > > > +      '(menu-item "Paragraph Indent Minor Mode"
> > >
> > > `Indent Paragraphs' - toggle button shows that it is modal
> > 
> > Could you clarify this at all? Do you just mean clearer labels or
> > could you give an example of a menu item that shows that the option is
> > modal?
> 
> Yes, suggestions for the labels.
> 
> Your code (which I did not try) indicates that you use toggle buttons: 
> :button (:toggle . (memq 'turn-on-auto-fill text-mode-hook))
> 
> They should thus appear similar to what we see in the Options menu.
> You can tell by looking at the menu item and its checkmark (or lack
> thereof) that the item represents something with two states ("modes"),
> and you can tell what the current state is.
> 
> This means that you need not add words in the menu item label to say
> that the thing is modal: "... Minor Mode".  (Yes, one cannot tell from
> the toggle button alone that the mode is minor, but that is only minor
> info. ;-))

Yes, I can see what you mean now. I'll be honest; I find the menu
implementations of all the major and minor modes to be frankly
horribly inconsistant. I suspect this is something I should get used
to.

--
John Anthony

RE: Text Mode Menu

[Drew Adams]

> I'll be honest; I find the menu implementations of all the major
> and minor modes to be frankly horribly inconsistant. I suspect
> this is something I should get used to.

No, I hope you don't. ;-)  That is, the fact that you notice such
inconsistency and it bothers you is a *good* thing for Emacs.

If you have the time, it is better to file a bug report/enhancement
request (`M-x report-emacs-bug'), preferably with some concrete
suggestions or even a patch or two, than it is to just adapt as a
user to any such perceived Emacs weaknesses.

It is good to find people particularly interested in the usability
of Emacs menus.  This has generally not been a super high priority
among Emacs developers.  But they will likely listen to your
suggestions.  But hurry, before you adapt to the status quo. ;-)

Re: Text Mode Menu

[John Anthony]

On Mon, Oct 07, 2013 at 01:44:10PM -0700, Drew Adams wrote:
> No, I hope you don't. ;-)  That is, the fact that you notice such
> inconsistency and it bothers you is a *good* thing for Emacs.

> It is good to find people particularly interested in the usability
> of Emacs menus.  This has generally not been a super high priority
> among Emacs developers.  But they will likely listen to your
> suggestions.  But hurry, before you adapt to the status quo. ;-)

Yes, it really seemed like a bit of a blind spot that I could make a
lot of fixes for quickly and learn about the codebase and internal
workings of major modes from.  Cheers for the positivity and
encouragement. I'll most likely submit my first patch tomorrow (sans
redundancy in labels) and have a bunch of good I can do while I build
to contributing something more substantive.

--
John Anthony