Re: Patch to ido.el

[Óscar Fuentes]

Hello Steven. Some comments below

Steven Degutis <address@hidden> writes:

> Disclaimer: I have absolutely no idea how this process works.
>
> This patch makes ido.el slightly more configurable.
>
> In particular, when using ido-decorations that make ido-mode display
> vertically (per
> http://emacswiki.org/emacs/InteractivelyDoThings#toc20), a

There is something wrong with that link. Tried it with Opera and Firefox
and, after entering the URL, both show the page's table of contents,
with the index line for toc20 highlighted as "visited", but you need to
click on the entry itself for jumping to the linked section. It must be
something with emacswiki.

Apart from that, the code contained on that section has a bug:

(defun ido-define-keys () ;; C-n/p is more intuitive in vertical layout
    (define-key ido-completion-map (kbd "C-n") 'ido-next-match)
    (define-key ido-completion-map (kbd "C-p") 'ido-prev-match))
(add-hook 'ido-setup-hook 'jf-ido-define-keys)

`jf-ido-define-keys' is not defined anywhere. I guess that it is
supposed to be `ido-define-keys'.

> pair of decorations are being used in two different places with two
> different meanings, even though by happenstance they look the same. This
> patch splits them out into two different pairs of decorations (which
> currently have the same value), allowing the user to customize them with
> greater precision.
>
>
> diff --git ido.el ido2.el
> index cda4021..dfb5b0a 100644
> --- ido.el
> +++ ido2.el
> @@ -763,7 +763,7 @@ Obsolete.  Set 3rd element of `ido-decorations'
> instead."
>    :type '(choice string (const nil))
>    :group 'ido)
>
> -(defcustom ido-decorations '( "{" "}" " | " " | ..." "[" "]" " [No match]"
> " [Matched]" " [Not readable]" " [Too big]" " [Confirm]")
> +(defcustom ido-decorations '( "{" "}" " | " " | ..." "[" "]" " [No match]"
> " [Matched]" " [Not readable]" " [Too big]" " [Confirm]" "[" "]")
>    "List of strings used by ido to display the alternatives in the
> minibuffer.
>  There are 11 elements in this list:
>  1st and 2nd elements are used as brackets around the prospect list,
> @@ -4570,9 +4570,9 @@ For details of keybindings, see `ido-find-file'."
>                                         (ido-name (car comps))))
>                         ""
>                       ;; when there is one match, show the matching file
> name in full
> -                     (concat (nth 4 ido-decorations)  ;; [ ... ]
> +                     (concat (nth 11 ido-decorations)  ;; [ ... ]
>                               (ido-name (car comps))
> -                             (nth 5 ido-decorations)))
> +                             (nth 12 ido-decorations)))
>     (if (not ido-use-faces) (nth 7 ido-decorations))))  ;; [Matched]
>    (t ;multiple matches
>     (let* ((items (if (> ido-max-prospects 0) (1+ ido-max-prospects) 999))
>
>
>
> So, now that I've said everything that can probably be said on the topic,
> how do I go about submitting this patch for review to the official emacs
> repo? Does this email count? Or maybe someone in here knows how to do it in
> like 3 seconds flat and wouldn't mind?

When submitting patches inline, please watch for line-wrapping. When in
doubt, attach the patch.

Also you should mention the base revision or version of the code you
used for creating your patch. In this case it seems to be against some
stable release instead of Emacs' development sources.

Usually the right place for submitting patches is the bug mailing list.
Just do M-x report-emacs-bug. This way it will not disappear in the
developer's mailing list chatter.

Finally, for significant modifications the FSF requires to sign a
document about copyright. I think that your modification is small enough
to not requiring that paperwork.

You might be interested on reading $EMACS/etc/CONTRIBUTE, where all this
is more is explained in detail.