From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: 12537@debbugs.gnu.org
Subject: bug#12537: Acknowledgement (support for git commit --amend/--signoff)
Date: Sun, 30 Sep 2012 23:16:05 -0400 [thread overview]
Message-ID: <jwv8vbqoq2j.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <50663D58.4010702@yandex.ru> (Dmitry Gutov's message of "Sat, 29 Sep 2012 04:14:16 +0400")
> - (,(concat "^\\(\\([[:alpha:]]+\\):\\)" log-edit-header-contents-regexp)
> + (,(concat "^\\(\\([[:alpha:]][^: \n\t]+\\):\\)"
> + log-edit-header-contents-regexp)
I'd prefer to only add hyphens, as in [[:alpha:]-].
> +(defun log-edit-toggle-header (name value)
> + "Toggle a boolean-type header in the current buffer.
> +If the value of NAME is VALUE, remove it. Otherwise, add it if
> +it's not present and set it to VALUE. Afterward, if there are headers,
> +make sure there is an empty line after them. If there are no headers,
> +remove all empty lines at the beginning of the buffer.
> +Return t if toggled on, otherwise nil."
How 'bout leaving the header, just with an empty content, so you never
have to deal with "remove a sole empty line if there's no header left"?
> +or (HEADER CMDARG VALUE) associating header names to the corresponding
> +cmdlineoption name and the result is then a list of the form
> +\(MSG CMDARG1 HDRTEXT1 CMDARG2 HDRTEXT2...\) where MSG is the remaining text
> +from STRING. For HEADERS elements of the second type, the header value is
> +not added to the list. And CMDARG is added to the result list only if
> +the header value is the same as VALUE.
I think I'd rather provide something a bit more general. E.g. accept
entries of the form (HEADER . FUNCTION) where function takes the
header's value and returns a list of arguments where vc-git can provide
as FUNCTION something like
(lambda (val) (if (equal val "yes") '("--amend")))
> +(defun vc-git-log-edit-toggle-signoff ()
> + (interactive)
> + (log-edit-toggle-header "Sign-Off" "yes"))
please provide a docstring for interactive functions.
> +(defun vc-git-log-edit-toggle-amend ()
> + (interactive)
Same here.
> +(define-derived-mode vc-git-log-edit-mode log-edit-mode "*VC-log*"
"*VC-log*"? Really? Shouldn't that be "Log-Edit" or "Log-Edit/git"
or something?
> + "Major mode for editing Git log messages.
> +It is based on `log-edit-mode', and has Git-specific extensions.
> +\\{vc-git-log-edit-mode-map}")
The \\{vc-git-log-edit-mode-map} shouldn't be needed since
define-derived-mode will add it for you anyway.
Other than that, it looks OK, so feel free to install it after you fixed
the above details.
Stefan
next prev parent reply other threads:[~2012-10-01 3:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-29 0:11 bug#12537: support for git commit --amend/--signoff Dmitry Gutov
[not found] ` <handler.12537.B.134887753227659.ack@debbugs.gnu.org>
2012-09-29 0:14 ` bug#12537: Acknowledgement (support for git commit --amend/--signoff) Dmitry Gutov
2012-10-01 3:16 ` Stefan Monnier [this message]
2012-10-01 3:59 ` Dmitry Gutov
2012-10-01 4:32 ` Chong Yidong
2012-10-02 0:28 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwv8vbqoq2j.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=12537@debbugs.gnu.org \
--cc=dgutov@yandex.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).