Re: EWMH package, please review.

["Jan D." <jan.h.d@swipnet.se>]

>> don't know if this is useful enough to be included in Emacs, so I'd
>> like your views on that also.
>
> I don't know.  Are (all of) those hints something an ordinary user
> would typically toggle interactively?

Shaded, fullscreen and maximize horizontal and vertical are.  Above and
below are probably a bit obscure.

>
>
>> ;;; Code:
>>
>> (defun x-ewmh-send (arg hint frame &optional hint2)
>>   "Send a client message to change an extended window manager hint.
>>
>> FRAME may be nil to use the selected frame.
>> If ARG is poitive, add HINT.
>> If ARG is negative, remove HINT.
>> Otherwise toggle HINT."
>>
>>   (let* ((eff-arg (if (null arg) 0 (prefix-numeric-value arg)))
>
> This is simpler:
>
>   (setq arg (if (null arg) 0 (prefix-numeric-value arg)))
>   (let ((action ...use ARG instead of EFF-ARG...))

I was told to avoid setq when I learned lisp :-).

>
> IMO, it is somewhat unusual to let a generic, non-interactive function
> assume that it is always invoked by an interactive function.  I would
> normally let the callers do that (possibly using an auxiliary
> function), but in your case I would just change the DOC string to say
> that ARG is supposed to be a raw prefix argument.

I'd just wanted to minimize the duplication.  I understand the problem
though, I'll change the doc.

>
>
>> 	 (action (cond ((= eff-arg 0) 2)	;; Toggle
>> 		       ((> eff-arg 0) 1)	;; Add
>> 		       (t 0))))			;; Remove
>>     (x-send-client-message nil 0 frame "_NET_WM_STATE" 32
>> 			   (list action
>> 				 hint
>> 				 (if hint2 hint2 0)
>
> simpler:  (or hint2 0)

Right.

>
>> 				 0))))
>>
>>
>> (defun x-ewmh-fullscreen (&optional arg frame)
>>   "Toggle FRAME fullscreen hint using extended window manager hints 
>> (EWMH).
>
> Since this is an interactive function, you should describe the normal
> interactive function in the "one-liner", which in your case would mean
> not referring to the FRAME argument.
>
>>
>> If FRAME is not given, the selected frame is used.
>> If ARG is poitive, add fullscreen hint.
>> If ARG is negative, remove fullscreen hint.
>
> Since this is an interactive function, you would normally say that
> using a positive command prefix would add the hint and a negative
> prefix would remove the hint, while no prefix toggles the hint.
>
> But it might be clearer if you say that a C-u prefix (write
> \\[universal-argument] in the doc string) adds the hint, while
> a M-- prefix (write \\[negative-argument] removes the hint.

I'll change, thanks.  I'd probably should say more what the functions
do in the one liner.  I.e "Make a frame fullscreen" or some such.

>
>> Otherwise toggle fullscreen hint.
>
> I suggest you write something like this in the doc string:
>
>   "Toggle EWMH fullscreen hint of selected frame.
>
> With \\[universal-argument] prefix arg, add fullscreen hint, and
> with \\[negative-argument] prefix arg, remove fullscreen hint.
>
> When called from a Lisp program, optional second arg FRAME specifies 
> the
> frame on which to apply the fullscreen hint."

Good suggestion.

>
>>
>> If fullscreen doesn't work with your window manager, try
>> `x-ewmh-horz-and-vert'.
>>
>> NOTE:  If the window manager does not support EWMH, this does 
>> nothing."
>>   (interactive "P")
>>   (x-ewmh-send arg "_NET_WM_STATE_FULLSCREEN" frame))
>>
>
>
>>
>> (defun x-ewmh-maximized_vert (&optional arg frame)
>
> x-ewmh-maximized-vertical  (don't use underscore and write name in 
> full)
>
>>
>> (defun x-ewmh-maximized_horz (&optional arg frame)
>
> x-ewmh-maximized-horizontal  (ditto)
>
>>
>> (defun x-ewmh-horz-and-vert (&optional arg frame)
>
> x-ewmh-horizontal-and-vertical  (ditto)
>
>
> Maybe in general, the functions should be named
>
>         x-ewmh-toggle-...
>
> to emphasize that their default operation is to toggle the hint.

That is better.

Thanks for the comments.

	Jan D.