Proposal: emacsclient --readonly

Proposal: emacsclient --readonly

[michael]

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

As distraction to our toddler temper-tantrum, I'd like to propose a
new feature for emacsclient.

I use emacsclient to open files frequently and I generally immediately
do M-x view-mode once the frame pops up.  I'd like to be able to open
the file directly in view-mode when I invoke emacsclient.

I'd be interested in any suggestions people have on how to accomplish
this using emacsclient currently.  In the absence of any ideas, I'd like
to propose the folowing patch which adds a `--readonly,-r' option to
emacsclient (and server.el) (I had originally called it --view-mode,-v but
getopt_long_only complained that it was ambiguous if abbreviated.)

I'm eager to hear any suggestions people might have to do this without
changing emacsclient.  Or maybe this is a reason to add a more generic
feature to emacsclient to enable/disable modes after a file is opened.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacsclient --readonly feature --]
[-- Type: text/x-diff, Size: 6848 bytes --]

*** trunk/lib-src/emacsclient.c	2013-03-13 19:48:00.017919000 -0400
--- my-work/lib-src/emacsclient.c	2013-03-13 19:48:16.237220000 -0400
***************
*** 129,134 ****
--- 129,137 ----
  /* Nonzero means don't wait for a response from Emacs.  --no-wait.  */
  int nowait = 0;
  
+ /* Nonzero means file should be readonly and placed in view-mode.  --readonly.  */
+ int readonly = 0;
+ 
  /* Nonzero means don't print messages for successful operations.  --quiet. */
  int quiet = 0;
  
***************
*** 173,178 ****
--- 176,182 ----
  struct option longopts[] =
  {
    { "no-wait",	no_argument,	   NULL, 'n' },
+   { "readonly",	no_argument,	   NULL, 'r' },
    { "quiet",	no_argument,	   NULL, 'q' },
    { "eval",	no_argument,	   NULL, 'e' },
    { "help",	no_argument,	   NULL, 'H' },
***************
*** 519,524 ****
--- 523,532 ----
  	  nowait = 1;
  	  break;
  
+ 	case 'r':
+ 	  readonly = 1;
+ 	  break;
+ 
  	case 'e':
  	  eval = 1;
  	  break;
***************
*** 644,649 ****
--- 652,658 ----
  			Set the parameters of a new frame\n\
  -e, --eval    		Evaluate the FILE arguments as ELisp expressions\n\
  -n, --no-wait		Don't wait for the server to return\n\
+ -r, --readonly		Make the buffer readonly and enable view-mode\n\
  -q, --quiet		Don't display messages on success\n\
  -d DISPLAY, --display=DISPLAY\n\
  			Visit the file in the given display\n\
***************
*** 1604,1609 ****
--- 1613,1621 ----
    if (nowait)
      send_to_emacs (emacs_socket, "-nowait ");
  
+   if (readonly)
+     send_to_emacs (emacs_socket, "-readonly ");
+ 
    if (current_frame)
      send_to_emacs (emacs_socket, "-current-frame ");
  
*** trunk/lisp/server.el	2013-03-02 22:36:33.309144000 -0500
--- my-work/lisp/server.el	2013-03-06 23:58:19.157146157 -0500
***************
*** 1050,1055 ****
--- 1050,1056 ----
  				    (or file-name-coding-system
  					default-file-name-coding-system)))
  		nowait     ; t if emacsclient does not want to wait for us.
+                 readonly   ; t to view files in read-only/view-mode
  		frame      ; Frame opened for the client (if any).
  		display    ; Open frame on this display.
  		parent-id  ; Window ID for XEmbed
***************
*** 1075,1080 ****
--- 1076,1084 ----
                  ;; -nowait:  Emacsclient won't wait for a result.
                  (`"-nowait" (setq nowait t))
  
+                 ;; -readonly:  place files in view-mode
+                 (`"-readonly" (setq readonly t))
+ 
                  ;; -current-frame:  Don't create frames.
                  (`"-current-frame" (setq use-current-frame t))
  
***************
*** 1233,1239 ****
                   (let ((default-directory
                           (if (and dir (file-directory-p dir))
                               dir default-directory)))
!                    (server-execute proc files nowait commands
                                     dontkill frame tty-name)))))
  
              (when (or frame files)
--- 1237,1243 ----
                   (let ((default-directory
                           (if (and dir (file-directory-p dir))
                               dir default-directory)))
!                    (server-execute proc files nowait readonly commands
                                     dontkill frame tty-name)))))
  
              (when (or frame files)
***************
*** 1243,1249 ****
      ;; condition-case
      (error (server-return-error proc err))))
  
! (defun server-execute (proc files nowait commands dontkill frame tty-name)
    ;; This is run from timers and process-filters, i.e. "asynchronously".
    ;; But w.r.t the user, this is not really asynchronous since the timer
    ;; is run after 0s and the process-filter is run in response to the
--- 1247,1253 ----
      ;; condition-case
      (error (server-return-error proc err))))
  
! (defun server-execute (proc files nowait readonly commands dontkill frame tty-name)
    ;; This is run from timers and process-filters, i.e. "asynchronously".
    ;; But w.r.t the user, this is not really asynchronous since the timer
    ;; is run after 0s and the process-filter is run in response to the
***************
*** 1253,1259 ****
    ;; including code that needs to wait.
    (with-local-quit
      (condition-case err
!         (let ((buffers (server-visit-files files proc nowait)))
            (mapc 'funcall (nreverse commands))
  
  	  ;; If we were told only to open a new client, obey
--- 1257,1263 ----
    ;; including code that needs to wait.
    (with-local-quit
      (condition-case err
!         (let ((buffers (server-visit-files files proc nowait readonly)))
            (mapc 'funcall (nreverse commands))
  
  	  ;; If we were told only to open a new client, obey
***************
*** 1319,1331 ****
        (when (> column-number 0)
          (move-to-column (1- column-number))))))
  
! (defun server-visit-files (files proc &optional nowait)
    "Find FILES and return a list of buffers created.
  FILES is an alist whose elements are (FILENAME . FILEPOS)
  where FILEPOS can be nil or a pair (LINENUMBER . COLUMNNUMBER).
  PROC is the client that requested this operation.
  NOWAIT non-nil means this client is not waiting for the results,
! so don't mark these buffers specially, just visit them normally."
    ;; Bind last-nonmenu-event to force use of keyboard, not mouse, for queries.
    (let ((last-nonmenu-event t) client-record)
      ;; Restore the current buffer afterward, but not using save-excursion,
--- 1323,1337 ----
        (when (> column-number 0)
          (move-to-column (1- column-number))))))
  
! (defun server-visit-files (files proc &optional nowait readonly)
    "Find FILES and return a list of buffers created.
  FILES is an alist whose elements are (FILENAME . FILEPOS)
  where FILEPOS can be nil or a pair (LINENUMBER . COLUMNNUMBER).
  PROC is the client that requested this operation.
  NOWAIT non-nil means this client is not waiting for the results,
! so don't mark these buffers specially, just visit them normally.
! READONLY non-nil means place the file in `view-mode'; quiting
! the file will kill the buffer."
    ;; Bind last-nonmenu-event to force use of keyboard, not mouse, for queries.
    (let ((last-nonmenu-event t) client-record)
      ;; Restore the current buffer afterward, but not using save-excursion,
***************
*** 1363,1368 ****
--- 1369,1379 ----
            (run-hooks 'server-visit-hook)
  	  ;; hooks may be specific to current buffer:
  	  (run-hooks 'post-command-hook))
+         (when readonly
+           ;; enable view mode and view-quit will mark the buffer as done
+           (view-mode +1)
+           (unless nowait
+             (setq view-exit-action 'server-buffer-done)))
  	(unless nowait
  	  ;; When the buffer is killed, inform the clients.
  	  (add-hook 'kill-buffer-hook 'server-kill-buffer nil t)

[-- Attachment #3: Type: text/plain, Size: 71 bytes --]


--
Michael R. Mauger  -  FSF Member #4247  -  Emacs sql.el Maintainer

Re: Proposal: emacsclient --readonly

[W. Greenhouse]

michael-ntT3PcgARwzQT0dZR+AlfA@public.gmane.org writes:

> As distraction to our toddler temper-tantrum, I'd like to propose a
> new feature for emacsclient.
>
> I use emacsclient to open files frequently and I generally immediately
> do M-x view-mode once the frame pops up.  I'd like to be able to open
> the file directly in view-mode when I invoke emacsclient.
>
> I'd be interested in any suggestions people have on how to accomplish
> this using emacsclient currently.  In the absence of any ideas, I'd like
> to propose the folowing patch which adds a `--readonly,-r' option to
> emacsclient (and server.el) (I had originally called it --view-mode,-v but
> getopt_long_only complained that it was ambiguous if abbreviated.)
>
> I'm eager to hear any suggestions people might have to do this without
> changing emacsclient.  Or maybe this is a reason to add a more generic
> feature to emacsclient to enable/disable modes after a file is opened.
>
>
>
> --
> Michael R. Mauger  -  FSF Member #4247  -  Emacs sql.el Maintainer

Without changing emacsclient, my approach would be the slightly
unintuitive/user-hostile

emacsclient --eval '(view-file "path/to/file")'

-- 
Regards,
WGG

Re: Proposal: emacsclient --readonly

[Stefan Monnier]

> I'd be interested in any suggestions people have on how to accomplish
> this using emacsclient currently.  In the absence of any ideas, I'd like
> to propose the folowing patch which adds a `--readonly,-r' option to
> emacsclient (and server.el) (I had originally called it --view-mode,-v but
> getopt_long_only complained that it was ambiguous if abbreviated.)

I think the way to do it is to change emacsclient.c so that any "--foo"
arg simply gets sent to server.el.  This way the addition of --readonly

support can be implemented all in server.el.  Then server.el should be
changed to provide a hook that lets user add handling for their
own --foo args so that you can even do it without changing server.el.

        Stefan

Re: Proposal: emacsclient --readonly

[Michael Mauger]

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

[I got a message this bounced on the mailing list, twice; trying again.
Sorry if this is a dupe.

I've polished the text and the code bit; it was originally sent while I
was attending LibrePlanet.]

> > I'd be interested in any suggestions people have on how to accomplish
> > this using emacsclient currently.  In the absence of any ideas, I'd like
> > to propose the folowing patch which adds a `--readonly,-r' option to
> > emacsclient (and server.el) (I had originally called it --view-mode,-v but
> > getopt_long_only complained that it was ambiguous if abbreviated.)
>
> I think the way to do it is to change emacsclient.c so that any "--foo"
> arg simply gets sent to server.el.  This way the addition of --readonly
>
> support can be implemented all in server.el.  Then server.el should be
> changed to provide a hook that lets user add handling for their
> own --foo args so that you can even do it without changing server.el.
>
>        Stefan

Adding a feature to allow custom options in emacsclient is an idea that
I've had in the past and failed at implementing it in the past miserably
because I came up with too many complex situations.  So I've spent the
last week or so trying again.  I've been through several iterations and
have reduced it to this simplified solution.  The code is still a
little rough, but the idea is stable.  The doc strings probably need
lots of work...

Basically, there are two variables used to control the parsing and
processing of custom emacsclient options:

server-custom-option-functions:
  A list of functions to handle custom options.

  The functions accept four arguments: a buffer, a process, the option
  string and the option value.  If this variable is nil, then no options
  are accepted, regardless of the setting of
  `server-custom-option-list'.  The functions on this hook variable are
  called once before files or expressions are processed with the buffer
  set to nil.  After each file is opened, the hooks will be called
  again, this time with the file buffer passed as the first argument.

server-custom-option-list:
  A list that defines the acceptable custom options.

  If this variable is nil, all unrecognized options are assumed to be
  valid custom options.  If this variable is a list then each element
  defines an acceptable option.  Each list entry should be either a
  string with the option name that does not accept a value, or a list
  whose first entry is a string containing the option name and the
  optional second entry is a type predicate.  If the type predicate is
  `string-only', then the option value is simply treated as a string;
  any other predicate forces the value to be interpreted by the elisp
  reader and passed to the predicate to validate it's value.


So, in my user-init-file, I have:

  (defun my-server-custom-options (buf proc option value)
    (if buf  ;; options processed for each file opened
        (pcase option
          (`"readonly"
           (view-mode +1)
           (when (and view-mode server-buffer-clients)  ;; not nowait
             (setq-local view-exit-action 'server-buffer-done)))
          (t nil))

      (pcase option ;; global options
        (`"banner"  ;; display banner value
         (cond
          ((executable-find "figlet")
           (call-process-shell-command "figlet" nil
                                       "*Banner*" t value))
          ((executable-find "banner")
           (call-process-shell-command "banner" nil
                                       "*Banner*" t value))
          (t (progn
               (get-buffer-create "*Banner*")
               (with-current-buffer "*Banner*"
                 (insert (propertize value 'face
                                     'font-lock-warning-face))))))
             (display-buffer "*Banner*"))
         (t nil))))

  ;; define option syntax
  (setq server-custom-option-list
        '("readonly"                ;; --readonly
          ("banner" string-only)))  ;; --banner=text

  ;; Process options
  (add-hook 'server-custom-option-functions 'my-server-custom-options)

In the end, the changes were pretty small and localized.  I have not
tested it that hard, so it's not difficult to break.

--
Michael R. Mauger       FSF Member #4247       Emacs sql.el Maintainer
michael # mauger com     mmaug # member fsf org     mmaug # yahoo com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: custom-option.diff --]
[-- Type: text/x-patch; name="custom-option.diff", Size: 15197 bytes --]

*** trunk/lib-src/emacsclient.c	2013-03-13 19:48:00.017919000 -0400
--- my-work/lib-src/emacsclient.c	2013-03-23 21:47:53.065402783 -0400
***************
*** 163,168 ****
--- 163,173 ----
  /* PID of the Emacs server process.  */
  int emacs_pid = 0;

+ /* Custom options */
+ #define MAX_CUSTOM_OPTIONS (32)
+ const char *custom_options[MAX_CUSTOM_OPTIONS];
+ int custopts = 0;
+
  /* If non-NULL, a string that should form a frame parameter alist to
     be used for the new frame */
  const char *frame_parameters = NULL;
***************
*** 473,478 ****
--- 478,484 ----
  {
    alternate_editor = egetenv ("ALTERNATE_EDITOR");

+   opterr = 0;                   /* don't display an error */
    while (1)
      {
        int opt = getopt_long_only (argc, argv,
***************
*** 554,559 ****
--- 560,574 ----
            frame_parameters = optarg;
            break;

+         case '?':
+           if (custopts < MAX_CUSTOM_OPTIONS)
+             custom_options [custopts++] = argv[optind-1];
+
+           else
+             message (TRUE, "%s: too many custom options: %s\n", progname, argv[optind-1]);
+
+           break;
+
  	default:
  	  message (TRUE, "Try `%s --help' for more information\n", progname);
  	  exit (EXIT_FAILURE);
***************
*** 660,666 ****
  "			If EDITOR is the empty string, start Emacs in daemon\n\
  			mode and try connecting again\n"
  #endif /* not WINDOWSNT */
! "\n\
  Report bugs with M-x report-emacs-bug.\n", progname);
    exit (EXIT_SUCCESS);
  }
--- 675,686 ----
  "			If EDITOR is the empty string, start Emacs in daemon\n\
  			mode and try connecting again\n"
  #endif /* not WINDOWSNT */
! "--XXX			Custom option XXX\n\
! --XXX=YYY		Custom option XXX with value YYY\n\
! 			See `server-custom-option-list' and
!                         `server-custom-option-functions' to setup \n\
!                         custom options\n\
! \n\
  Report bugs with M-x report-emacs-bug.\n", progname);
    exit (EXIT_SUCCESS);
  }
***************
*** 1549,1555 ****
    /* Process options.  */
    decode_options (argc, argv);

!   if ((argc - optind < 1) && !eval && current_frame)
      {
        message (TRUE, "%s: file name or argument required\n"
  	       "Try `%s --help' for more information\n",
--- 1569,1575 ----
    /* Process options.  */
    decode_options (argc, argv);

!   if ((argc - optind < 1) && !eval && !custopts && current_frame)
      {
        message (TRUE, "%s: file name or argument required\n"
  	       "Try `%s --help' for more information\n",
***************
*** 1651,1656 ****
--- 1671,1693 ----
    if (!current_frame && !tty)
      send_to_emacs (emacs_socket, "-window-system ");

+   if (custopts)
+     {
+       int i;
+       const char *opt;
+       for (i = 0; i < custopts; ++i)
+         {
+           /* Skip leading hyphens */
+           for (opt = custom_options[i]; *opt == '-'; ++opt)
+             ;
+
+           /* Send custom options to the server */
+           send_to_emacs (emacs_socket, "-custom ");
+           quote_argument (emacs_socket, opt);
+           send_to_emacs (emacs_socket, " ");
+         }
+     }
+
    if ((argc - optind > 0))
      {
        int i;
*** trunk/lisp/server.el	2013-03-02 22:36:33.309144000 -0500
--- my-work/lisp/server.el	2013-03-23 21:02:22.090413274 -0400
***************
*** 262,267 ****
--- 262,302 ----
    :type 'string
    :version "23.1")

+ ;; Custom options
+ (defcustom server-custom-option-functions nil
+   "A list of functions to handle custom options.
+ The functions accept four arguments: a buffer, a process, the
+ option string and the option value.  If this variable is nil,
+ then no options are accepted, regardless of the setting of
+ `server-custom-option-list'.  The functions on this hook variable
+ are called once before files or expressions are processed with
+ the buffer set to nil.  After each file is opened, the hooks will
+ be called again, this time with the file buffer passed as the
+ first argument."
+   :group 'server
+   :type 'hook
+   :version "24.4")
+
+ (defcustom server-custom-option-list nil
+   "A list that defines the acceptable custom options.
+ If this variable is nil, all unrecognized options are assumed to
+ be valid custom options.  If this variable is a list then each
+ element defines an acceptable option.  Each list entry should be
+ either a string with the option name that does not accept a
+ value, or a list whose first entry is a string containing the
+ option name and the optional second entry is a type predicate.
+ If the type predicate is `string-only', then the option value is
+ simply treated as a string; any other predicate forces the value
+ to be interpreted by the elisp reader and passed to the predicate
+ to validate it's value. "
+   :group 'server
+   :type '(repeat (choice (string :tag "Option")
+                          (list   :tag "Option=Parameter"
+                                  (string :tag "Option")
+                                  (choice (const :tag "String-only" string-only)
+                                          (function :tag "Parameter predicate")))))
+   :version "24.4")
+
  ;; We do not use `temporary-file-directory' here, because emacsclient
  ;; does not read the init file.
  (defvar server-socket-dir
***************
*** 955,960 ****
--- 990,998 ----
    Go to the given line and column number
    in the next file opened.

+ `-custom OPTION[=PARM]
+   Invoke a custom option.
+
  `-file FILENAME'
    Load the given file in the current frame.

***************
*** 1055,1060 ****
--- 1093,1099 ----
  		parent-id  ; Window ID for XEmbed
  		dontkill   ; t if client should not be killed.
  		commands
+ 		customs
  		dir
  		use-current-frame
  		frame-parameters  ;parameters for newly created frame
***************
*** 1157,1162 ****
--- 1196,1215 ----
                                 (string-to-number (or (match-string 2 arg)
                                                       ""))))))

+                 ;; -custom OPTION[=PARM]
+                 ;;  invoke custom lisp code
+                 (`"-custom"
+                  (let ((option (pop args-left)))
+                    (if (not server-custom-option-functions)
+                        (error "Custom options are not accepted --%s" option)
+
+                      (if coding-system
+                          (setq option (decode-coding-string option coding-system)))
+                      (server-log (format "Custom: %s" option) proc)
+
+                      (push (server-custom-option-parse option) customs)))
+                    (setq filepos nil))
+
                  ;; -file FILENAME:  Load the given file.
                  (`"-file"
                   (let ((file (pop args-left)))
***************
*** 1233,1239 ****
                   (let ((default-directory
                           (if (and dir (file-directory-p dir))
                               dir default-directory)))
!                    (server-execute proc files nowait commands
                                     dontkill frame tty-name)))))

              (when (or frame files)
--- 1286,1292 ----
                   (let ((default-directory
                           (if (and dir (file-directory-p dir))
                               dir default-directory)))
!                    (server-execute proc files nowait commands customs
                                     dontkill frame tty-name)))))

              (when (or frame files)
***************
*** 1243,1249 ****
      ;; condition-case
      (error (server-return-error proc err))))

! (defun server-execute (proc files nowait commands dontkill frame tty-name)
    ;; This is run from timers and process-filters, i.e. "asynchronously".
    ;; But w.r.t the user, this is not really asynchronous since the timer
    ;; is run after 0s and the process-filter is run in response to the
--- 1296,1302 ----
      ;; condition-case
      (error (server-return-error proc err))))

! (defun server-execute (proc files nowait commands customs dontkill frame tty-name)
    ;; This is run from timers and process-filters, i.e. "asynchronously".
    ;; But w.r.t the user, this is not really asynchronous since the timer
    ;; is run after 0s and the process-filter is run in response to the
***************
*** 1253,1265 ****
    ;; including code that needs to wait.
    (with-local-quit
      (condition-case err
!         (let ((buffers (server-visit-files files proc nowait)))
            (mapc 'funcall (nreverse commands))

  	  ;; If we were told only to open a new client, obey
  	  ;; `initial-buffer-choice' if it specifies a file
            ;; or a function.
!           (unless (or files commands)
              (let ((buf
                     (cond ((stringp initial-buffer-choice)
  			  (find-file-noselect initial-buffer-choice))
--- 1306,1318 ----
    ;; including code that needs to wait.
    (with-local-quit
      (condition-case err
!         (let ((buffers (server-visit-files files customs proc nowait)))
            (mapc 'funcall (nreverse commands))

  	  ;; If we were told only to open a new client, obey
  	  ;; `initial-buffer-choice' if it specifies a file
            ;; or a function.
!           (unless (or files commands customs)
              (let ((buf
                     (cond ((stringp initial-buffer-choice)
  			  (find-file-noselect initial-buffer-choice))
***************
*** 1319,1333 ****
        (when (> column-number 0)
          (move-to-column (1- column-number))))))

! (defun server-visit-files (files proc &optional nowait)
    "Find FILES and return a list of buffers created.
  FILES is an alist whose elements are (FILENAME . FILEPOS)
  where FILEPOS can be nil or a pair (LINENUMBER . COLUMNNUMBER).
  PROC is the client that requested this operation.
  NOWAIT non-nil means this client is not waiting for the results,
! so don't mark these buffers specially, just visit them normally."
    ;; Bind last-nonmenu-event to force use of keyboard, not mouse, for queries.
    (let ((last-nonmenu-event t) client-record)
      ;; Restore the current buffer afterward, but not using save-excursion,
      ;; because we don't want to save point in this buffer
      ;; if it happens to be one of those specified by the server.
--- 1372,1391 ----
        (when (> column-number 0)
          (move-to-column (1- column-number))))))

! (defun server-visit-files (files customs proc &optional nowait)
    "Find FILES and return a list of buffers created.
  FILES is an alist whose elements are (FILENAME . FILEPOS)
  where FILEPOS can be nil or a pair (LINENUMBER . COLUMNNUMBER).
  PROC is the client that requested this operation.
  NOWAIT non-nil means this client is not waiting for the results,
! so don't mark these buffers specially, just visit them normally.
!
! CUSTOMS are run within the context of the buffer of the opened
! file."
    ;; Bind last-nonmenu-event to force use of keyboard, not mouse, for queries.
    (let ((last-nonmenu-event t) client-record)
+     ;; Invoke custom options at the process level
+     (server-custom-option-call nil proc customs)
      ;; Restore the current buffer afterward, but not using save-excursion,
      ;; because we don't want to save point in this buffer
      ;; if it happens to be one of those specified by the server.
***************
*** 1367,1372 ****
--- 1425,1434 ----
  	  ;; When the buffer is killed, inform the clients.
  	  (add-hook 'kill-buffer-hook 'server-kill-buffer nil t)
  	  (push proc server-buffer-clients))
+         ;; Wait until here to invoke custom options so all the server
+         ;; related stuff is established.  But doing this earlier
+         ;; before some hooks can be justified as well.
+         (server-custom-option-call (current-buffer) proc customs)
  	(push (current-buffer) client-record)))
      (unless nowait
        (process-put proc 'buffers
***************
*** 1510,1515 ****
--- 1572,1642 ----
  	     (when server-process
  	       (server-buffer-done (current-buffer) t))))))
  \f
+ (defun server-custom-option-parse-1 (option svalue pred)
+   ;; We have encountered an OPTION with a string value SVALUE.  If
+   ;; PRED is non-nil, then a value is expected; if nil, then no value
+   ;; is accepted.  If PRED is `string-only, then the VALUE is left as
+   ;; a string; otherwise VALUE is run through the elisp parser and
+   ;; converted to elisp types.  If PRED is t, then any value is
+   ;; acceptable, otherwise the PRED is a single operand predicate to
+   ;; verify the data type of the parsed value is acceptable.
+   (let (value)
+     ;; We can't supply a value if there is no predicate
+     (when (and (not pred) svalue)
+       (error "Custom option --%s does not accept a value" option))
+     ;; Is there a value?
+     (if (or svalue (< 0 (length svalue)))
+         ;; We have a string value, is that all we want
+         (if (eq pred 'string-only)
+             (cons option svalue)
+           ;; We want a lisp value, parse it make sure we got it all
+           (setq value (read-from-string svalue))
+           (when (> (length svalue) (cdr value))
+             (error "Invalid custom option value --%s=%s" option svalue))
+           (setq value (car value))
+           ;; Validate it
+           (if (or (eq pred t)
+                   (funcall pred value))
+               (cons option value)
+             (error "Custom option --%s expected a(n) `%s' value, got `%s'"
+                    option pred value)))
+       ;; Treat nil and empty strings as a nil value
+       (cons option nil))))
+
+ (defun server-custom-option-parse (option-value)
+   (let (option svalue result)
+     (if (not (string-match "\\`\\([^=]+\\)\\(?:[=]\\(.*\\)\\)?\\'" option-value))
+         (error "Invalid custom option format --%s" option-value)
+       (setq option (match-string 1 option-value)
+             svalue (match-string 2 option-value))
+       ;; look-up the option
+       (if (not server-custom-option-list)
+           (setq result (cons option svalue))
+         (dolist (o server-custom-option-list)
+           (cond
+            ;; defn is a string, expect no value
+            ((and (stringp o)
+                  (string= o option))
+             (setq result
+                   (server-custom-option-parse-1 option svalue nil)))
+            ;; defn is a list, expect a value, second elem is type
+            ((and (consp o)
+                  (string= (car o) option))
+             (setq result
+                   (server-custom-option-parse-1 option svalue (or (cadr o) t))))))))
+     (if result
+         result
+       (error "Unrecognized custom option --%s" option))))
+
+ (defun server-custom-option-call (buf proc customs)
+   (server-log (format "server-custom-option-call: `%s'"
+                       (or (buffer-name buf) (process-name proc))) proc)
+   (with-local-quit
+     (dolist (c customs)
+       (run-hook-with-args 'server-custom-option-functions
+                           buf proc (car c) (cdr c)))))
+
+ \f
  (defun server-edit (&optional arg)
    "Switch to next server editing buffer; say \"Done\" for current buffer.
  If a server buffer is current, it is marked \"done\" and optionally saved.

Re: Proposal: emacsclient --readonly

[Michael Mauger]

----- Original Message -----
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> 
>>  server-custom-option-functions:
> [...]
>>  server-custom-option-list nil
> 
> How 'bout a `server-custom-option-function' which takes the list of
> arguments and returns a new list of arguments?
> 
> 
>         Stefan


I don't understand your comment--the `server-custom-option-list' 
variable is the list of options for parsing; the 
`server-custom-option-functions' variable is the list of functions to 
handle the options.  I can certainly see how we can merge these two
but I'm still confused by your comment.

If we were to merge the variables, we could end up with a list with 
entries like the following:

   ("OPTION" {PRED|t|nil|string-only} HANDLER)

I don't understand what arguments "takes the list of arguments and 
returns a new list of arguments" you are referring to?  Do you mean
parsing the command line args and removing the ones that it is 
processing?  I had looked at that a little, but figured I'd let the parsing
be done in C in emacsclient and just forward the unrecognized to lisp.
But even if we assemble the custom args into a list and try to handle 
them in lisp, how does that simplify the lisp side of the fence? 

Thanks, Michael

Re: Proposal: emacsclient --readonly

[Stefan Monnier]

> I don't understand your comment--the `server-custom-option-list' 
> variable is the list of options for parsing; the 
> `server-custom-option-functions' variable is the list of functions to 
> handle the options.  I can certainly see how we can merge these two
> but I'm still confused by your comment.

> If we were to merge the variables, we could end up with a list with 
> entries like the following:

>    ("OPTION" {PRED|t|nil|string-only} HANDLER)

My proposal is that you don't declare which function handles
which argument.  Just use a single server-custom-option-function which
is called regardless of which arguments were received and can do
anything it likes with them.

> I don't understand what arguments "takes the list of arguments and 
> returns a new list of arguments" you are referring to?  Do you mean
> parsing the command line args and removing the ones that it is 
> processing?

Yes.

>  I had looked at that a little, but figured I'd let the parsing
> be done in C in emacsclient and just forward the unrecognized to lisp.

Hmm... I don't understand what you're saying here.  I'm only talking
about the server.el side, and AFAICT you also were only talking about
the server.el side, since the client side does not have access to
Elisp customizations.

But yes, clearly the C side would do the parsing it does now, except
that it would additionally pass through any unrecognized argument.

> But even if we assemble the custom args into a list and try to handle
> them in lisp, how does that simplify the lisp side of the fence?

The server.el code already turns the string received from emacsclient
into a list of strings.  My suggestion is to try and limit the server.el
change to something like the patch below.


        Stefan


=== modified file 'lisp/server.el'
--- lisp/server.el	2013-02-13 04:31:09 +0000
+++ lisp/server.el	2013-03-25 13:07:44 +0000
@@ -909,6 +909,12 @@
     (process-put proc 'continuation nil)
     (if continuation (ignore-errors (funcall continuation)))))
 
+(defvar server-custom-option-function #'identity
+  "Function to process additional emacsclient arguments.
+The function is called with a single argument (a list of args received
+from emacsclient) and returns the list of args left to process.
+The easiest way to modify this variable is through `add-function'.")
+
 (cl-defun server-process-filter (proc string)
   "Process a request from the server to edit some files.
 PROC is the server process.  STRING consists of a sequence of
@@ -1067,7 +1076,8 @@
 	    (setq string (substring string (match-end 0)))
 	    (setq args-left
 		  (mapcar 'server-unquote-arg (split-string request " " t)))
-	    (while args-left
+	    (while (setq arg-left (funcall server-custom-option-function
+                                           args-left))
               (pcase (pop args-left)
                 ;; -version CLIENT-VERSION: obsolete at birth.
                 (`"-version" (pop args-left))

Re: Proposal: emacsclient --readonly

[Michael Mauger]

> From: Stefan Monnier <monnier@iro.umontreal.ca>

> 
> My suggestion is to try and limit the server.el change to
> something like the patch below.
> 
> === modified file 'lisp/server.el'
> --- lisp/server.el    2013-02-13 04:31:09 +0000
> +++ lisp/server.el    2013-03-25 13:07:44 +0000
> @@ -909,6 +909,12 @@
>      (process-put proc 'continuation nil)
>      (if continuation (ignore-errors (funcall continuation)))))
> 
> +(defvar server-custom-option-function #'identity
> +  "Function to process additional emacsclient arguments.
> +The function is called with a single argument (a list of args received
> +from emacsclient) and returns the list of args left to process.
> +The easiest way to modify this variable is through `add-function'.")
> +
> (cl-defun server-process-filter (proc string)
>    "Process a request from the server to edit some files.
> PROC is the server process.  STRING consists of a sequence of
> @@ -1067,7 +1076,8 @@
>         (setq string (substring string (match-end 0)))
>         (setq args-left
>           (mapcar 'server-unquote-arg (split-string request " " 
> t)))
> -        (while args-left
> +        (while (setq arg-left (funcall server-custom-option-function
> +                                           args-left))
>                (pcase (pop args-left)
>                  ;; -version CLIENT-VERSION: obsolete at birth.
>                  (`"-version" (pop args-left))
> 

So with this structure, how would I implement the --readonly custom
argument?

My guess is that the server-custom-option-function would scan the
incoming list of options for the -nowait entry because that will
determine our exit action.  However, we will leave the entry in place
because it needs the standard processing.  We will then look for the
--readonly entry and record that we want to view files rather than
edit them.  We will then remove the --readonly entry from the list to
allow the remainder of the processing to continue.

We can only record these settings at this point because the files have
not been actually opened yet.  And then we need to attach ourselves to
the server-visit-hook possibly to actually activate the readonly
behavior.  Or we could essentially duplicate the server processing by
consuming all of the arguments and replicating their actions with the
wrinkles we wish to introduce which is obviously not a great solution.

So it looks with the solution may be more flexible but requires global
variables specific to the feature and two hook functions to implement
it. My goal was for a solution the declares the options we want to add
and somewhat automate the parsing and validation of the option value
so that custom options behave as other command line arguments do.  It
then provides a handler function that implements the feature either
globally or on a per-buffer basis.  With this design, features could
be added to emacsclient by loading a library and adding an entry to a
list that defines the syntax and handling.

I have reworked my code to use a single list with option-name, handler
function, and value-predicate.  The result is that we could implement
modules in ELPA that would enhance emacsclient with new options
without requiring the modules to be integrated together into a single
function.  The readonly feature becomes about 10 lines of code; easily
maintained and separate from any other features.

-- Michael

Re: Proposal: emacsclient --readonly

[Stefan Monnier]

> So with this structure, how would I implement the --readonly custom
> argument?

Good question.

> My guess is that the server-custom-option-function would scan the
> incoming list of options for the -nowait entry because that will
> determine our exit action.

Hmm... why do we care about -nowait?

> We will then look for the --readonly entry and record that we want to
> view files rather than edit them.  We will then remove the --readonly
> entry from the list to allow the remainder of the processing
> to continue.

Right, we check (equal "--readonly" (car args)) and if non-nil, we (pop
args) and somehow register the "put in read-only mode" somewhere.

> behavior.  Or we could essentially duplicate the server processing by
> consuming all of the arguments and replicating their actions with the
> wrinkles we wish to introduce which is obviously not a great solution.

Right, we don't want to go there.

> So it looks with the solution may be more flexible but requires global
> variables specific to the feature and two hook functions to implement
> it. My goal was for a solution the declares the options we want to add
> and somewhat automate the parsing and validation of the option value
> so that custom options behave as other command line arguments do.  It
> then provides a handler function that implements the feature either
> globally or on a per-buffer basis.  With this design, features could
> be added to emacsclient by loading a library and adding an entry to a
> list that defines the syntax and handling.

Obviously, my proposal is incomplete indeed because it does not address
the need to "register the `put in read-only mode' somewhere".

But I think your design is too narrowly constrained by the needs you see
now.  I think we should aim for a design that could at least accommodate
some of the features currently hard-coded such as --eval.

Ideally it should also accommodate something like "--diff FILE1 FILE2"
which would call `diff' on the two files and might even be made to
accept "--diff FILE1 FILE2 FILE3" to do a 3-way merge.


        Stefan

Re: Proposal: emacsclient --readonly

[Michael Mauger]

> From: Stefan Monnier <monnier@iro.umontreal.ca>

>
>>  My guess is that the server-custom-option-function would scan the
>>  incoming list of options for the -nowait entry because that will
>>  determine our exit action.
> 
> Hmm... why do we care about -nowait?
> 

If there is an emacsclient waiting for us to finish, then we have to call
server-buffer-done.

> But I think your design is too narrowly constrained by the needs you see
> now.  I think we should aim for a design that could at least accommodate
> some of the features currently hard-coded such as --eval.
> 
> Ideally it should also accommodate something like "--diff FILE1 FILE2"
> which would call `diff' on the two files and might even be made to
> accept "--diff FILE1 FILE2 FILE3" to do a 3-way merge.
> 

 Okay at least we're sorta on the same page.  I had been thinking about
--diff at one point and the issues got very messy fast.  So here's my
thinking, let me know if you think I should continue:

In the C code, if there are any unrecognized options they will be sent over 
as a -custom option followed by the non-option parameters as -args

On the server side, the elisp will call the handler for each -custom option 
and pass the args along with it.  The handler can pull off the args it needs.  
If there are any arguments left over, then they will be passed either to 
--eval or treated as files.

The other required change will be to break out some of the file processing
so that the logic can be reused in custom options. 

* file-name generation

* visit a file

This will make the change more complex but won't really alter the 
behavior, just refactor it so it's more useful to custom options.

I should have a prototype in a day or two, unless you think I'm still 
missing the mark.

-- Michael

Re: Proposal: emacsclient --readonly

[Stefan Monnier]

> In the C code, if there are any unrecognized options they will be sent over 
> as a -custom option followed by the non-option parameters as -args

I'm not sure I understand right, but it doesn't sound like it's going
a direction I like: I'd like to reduce the difference between the
C-level command line argument vector received by emacsclient and the
list of strings received by server.el.

> On the server side, the elisp will call the handler for each -custom option 
> and pass the args along with it.

The --diff and --eval options don't take just one "argument", which is
why I don't want to hardcode this idea of a "--option ARG" format.

> The other required change will be to break out some of the file processing
> so that the logic can be reused in custom options. 

> * file-name generation

> * visit a file

> This will make the change more complex but won't really alter the 
> behavior, just refactor it so it's more useful to custom options.

Refactoring sounds fine.


        Stefan

Re: Proposal: emacsclient --readonly

[Michael Mauger]

> 

>>  In the C code, if there are any unrecognized options they will be sent 
> over 
>>  as a -custom option followed by the non-option parameters as -args
> 
> I'm not sure I understand right, but it doesn't sound like it's 
> going
> a direction I like: I'd like to reduce the difference between the
> C-level command line argument vector received by emacsclient and the
> list of strings received by server.el.
> 

Not sure how this is possible because the parameters are not
sent as they are encountered.  Based on the settings the client
programs behavior is altered so I will try to accomplish this as
best as I can.

>>  On the server side, the elisp will call the handler for each -custom 
> option 
>>  and pass the args along with it.
> 
> The --diff and --eval options don't take just one "argument", 
> which is
> why I don't want to hardcode this idea of a "--option ARG" format.
> 


I clearly wasn't very eloquent  last nite, in fact, after reading what I 
wrote, I'm not sure I knew what I meant.  I started to hack at this 
and I think I can describe it with more clarity now:

In the C code, we use getopt_long_only to parse the command 
line args, so I want to make sure that the custom options behave
as the built-in options do, so there is some complexity here.

If an option is encountered that is not recognized, then we will 
parse the argument string to see if it is in the format OPTION=ARG,
if it is, then we send the following "-custom OPTION -arg ARG".
In this case, that is the only option value and parsing continues
with the next comand line arg.  

If the option lacks an equal sign, then the option is sent with
"-custom OPTION", it then scans thru the following arguments 
and any that don't start with a hyphen will be sent as "-arg XXX"
so that there may be many -arg entries following the -custom.

The danger, of course, is that we may associate an arg with an
option because the C program knows nothing about the option
or the number of args it takes.  So the elisp side will have to take
any unused args and treat them as either -file or -eval entries.
There be dragons here.  If the --eval option was specified, the flag
is not sent so the elisp side doesn't know to treat the unused args 
as expressions rather than file names unless there are other non-
option arguments supplied.  This is going to require some addl
info being sent to elisp but it's not a real complication.

Here's an example:

    emacsclient --eval --my-new-option A B C

Sends:
    -... standard entries dispay, dir, env, et al
    -custom my-new-option -arg A -arg B -arg C

If my-new-option requires two args then it consumes A and B.

This leaves C as a unused entry.  Since there are no -file or 
-eval entries we don't know how to handle the unused unless
there is a separate entry to indicate how unused args should
be handled.

The ambiguity can be addressed by specifying the arguments 
as:

    emacsclient --eval C --my-new-option A B
or
    emacsclient --my-new-option A B --eval C

There may be some issues since the options and parameters
are not sent to the elisp code in the same order they are 
specified.  This isn't easy to correct so I expect that some 
custom options may have unintended behavior because the
sequence of file-opens or expression-evaluations may not 
happen in the same order.  

> 
> Refactoring sounds fine.
> 


I hope this clarifies things a bit.  As I said, I've got some of 
the code ready and should wrap it up tonite.  I'll then try to
implement --readonly and --diff to demonstrate how this will
all look.

-- Michael

Re: Proposal: emacsclient --readonly

[Michael Mauger]

>>  I'm not sure I understand right, but it doesn't sound like it's 

>>  going a direction I like: I'd like to reduce the difference between
>>  the C-level command line argument vector received by 
>> emacsclient and the list of strings received by server.el.
>>  

I'm giving up.  Without a significant rewrite of emacsclient.c
and server.el, I don't see your vision of custom options coming
to fruition.  

I am running a local edit similar to the second version I sent
to the mailing list that supports --readonly, --diff, --merge, 
and a little more invasive patch that implements passing a 
default Tramp prefix to allow a remote editor to open the 
local file.

The patch I use has:

* separate handlers for each option with built-in
parsing and validation of a single option argument

* a hook for handling each non-option parameter of emacsclient
to alter the filename before the filename is opened

* a hook to replace the default open action of each file arg

* a hook to handle each buffer that gets opened; similar to
`server-visit-hook' but called later once all server setup is
complete and dedicated to custom options.

* a final hook at the end of the file args to validate the overall
server state before control is passed to the user.

* allows custom options to be defined, maintained and installed
independently of each other without a point of contact beyond 
server.el (this is important for me since the custom features
I implement at home differ from those I might use at a client
site.)

As I tried to implement the simplified parsing that was proposed
but encountered the existing server processing which lacks the 
structure to implement the requested features.  The patch
that I am using takes advantage of several new hooks that allow
the new functionality to be inserted into the existing processing 
model.

The challenge for me will now be for me to keep my patch in sync.

-- Michael

Re: Proposal: emacsclient --readonly

[Rüdiger Sonderfeld]

Hello,
what happened to this patch?  It would be very useful to me right now.

There should probably be a way to add functions to `commands' (in server-
process-filter) though.  With this patch it could be done by adding an "--
eval" statement to the args.  But there should probably be a clean way.

I'm currently attempting to write an info(1) like program which loads the info 
page in the running Emacs session.  I need this to support the "doc" comment 
in `inferior-octave'.  With this patch it could be done without a lot of 
hacks.

Regards,
Rüdiger

> === modified file 'lisp/server.el'
> --- lisp/server.el      2013-02-13 04:31:09 +0000
> +++ lisp/server.el      2013-03-25 13:07:44 +0000
> @@ -909,6 +909,12 @@
> 
>      (process-put proc 'continuation nil)
>      (if continuation (ignore-errors (funcall continuation)))))
> 
> +(defvar server-custom-option-function #'identity
> +  "Function to process additional emacsclient arguments.
> +The function is called with a single argument (a list of args received
> +from emacsclient) and returns the list of args left to process.
> +The easiest way to modify this variable is through `add-function'.")
> +
> 
>  (cl-defun server-process-filter (proc string)
>  
>    "Process a request from the server to edit some files.
>  
>  PROC is the server process.  STRING consists of a sequence of
> 
> @@ -1067,7 +1076,8 @@
> 
>             (setq string (substring string (match-end 0)))
>             (setq args-left
>             
>                   (mapcar 'server-unquote-arg (split-string request " " t)))
> 
> -           (while args-left
> +           (while (setq arg-left (funcall server-custom-option-function
> +                                           args-left))
> 
>                (pcase (pop args-left)
>                
>                  ;; -version CLIENT-VERSION: obsolete at birth.
>                  (`"-version" (pop args-left))