all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Text Mode Menu
@ 2013-10-05  2:14 John Anthony
  2013-10-05 14:24 ` Drew Adams
  0 siblings, 1 reply; 9+ messages in thread
From: John Anthony @ 2013-10-05  2:14 UTC (permalink / raw)
  To: emacs-devel

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



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

* RE: Text Mode Menu
  2013-10-05  2:14 Text Mode Menu John Anthony
@ 2013-10-05 14:24 ` Drew Adams
  2013-10-07 15:47   ` John Anthony
  0 siblings, 1 reply; 9+ messages in thread
From: Drew Adams @ 2013-10-05 14:24 UTC (permalink / raw)
  To: John Anthony, emacs-devel

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



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

* Re: Text Mode Menu
  2013-10-05 14:24 ` Drew Adams
@ 2013-10-07 15:47   ` John Anthony
  2013-10-07 17:22     ` Drew Adams
  0 siblings, 1 reply; 9+ messages in thread
From: John Anthony @ 2013-10-07 15:47 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

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.



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

* RE: Text Mode Menu
  2013-10-07 15:47   ` John Anthony
@ 2013-10-07 17:22     ` Drew Adams
  2013-10-07 17:41       ` Davis Herring
  2013-10-07 19:41       ` John Anthony
  0 siblings, 2 replies; 9+ messages in thread
From: Drew Adams @ 2013-10-07 17:22 UTC (permalink / raw)
  To: John Anthony; +Cc: emacs-devel


> > 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. ;-))



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

* Re: Text Mode Menu
  2013-10-07 17:22     ` Drew Adams
@ 2013-10-07 17:41       ` Davis Herring
  2013-10-07 17:49         ` Drew Adams
  2013-10-07 19:41       ` John Anthony
  1 sibling, 1 reply; 9+ messages in thread
From: Davis Herring @ 2013-10-07 17:41 UTC (permalink / raw)
  To: Drew Adams; +Cc: John Anthony, emacs-devel

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



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

* RE: Text Mode Menu
  2013-10-07 17:41       ` Davis Herring
@ 2013-10-07 17:49         ` Drew Adams
  0 siblings, 0 replies; 9+ messages in thread
From: Drew Adams @ 2013-10-07 17:49 UTC (permalink / raw)
  To: Davis Herring; +Cc: John Anthony, emacs-devel

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



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

* Re: Text Mode Menu
  2013-10-07 17:22     ` Drew Adams
  2013-10-07 17:41       ` Davis Herring
@ 2013-10-07 19:41       ` John Anthony
  2013-10-07 20:44         ` Drew Adams
  1 sibling, 1 reply; 9+ messages in thread
From: John Anthony @ 2013-10-07 19:41 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

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



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

* RE: Text Mode Menu
  2013-10-07 19:41       ` John Anthony
@ 2013-10-07 20:44         ` Drew Adams
  2013-10-07 21:52           ` John Anthony
  0 siblings, 1 reply; 9+ messages in thread
From: Drew Adams @ 2013-10-07 20:44 UTC (permalink / raw)
  To: John Anthony; +Cc: emacs-devel

> 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. ;-)



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

* Re: Text Mode Menu
  2013-10-07 20:44         ` Drew Adams
@ 2013-10-07 21:52           ` John Anthony
  0 siblings, 0 replies; 9+ messages in thread
From: John Anthony @ 2013-10-07 21:52 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

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



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

end of thread, other threads:[~2013-10-07 21:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-05  2:14 Text Mode Menu John Anthony
2013-10-05 14:24 ` Drew Adams
2013-10-07 15:47   ` John Anthony
2013-10-07 17:22     ` Drew Adams
2013-10-07 17:41       ` Davis Herring
2013-10-07 17:49         ` Drew Adams
2013-10-07 19:41       ` John Anthony
2013-10-07 20:44         ` Drew Adams
2013-10-07 21:52           ` John Anthony

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.