[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#12537: Acknowledgement (support for git commit --amend/--signoff)
From: |
Stefan Monnier |
Subject: |
bug#12537: Acknowledgement (support for git commit --amend/--signoff) |
Date: |
Sun, 30 Sep 2012 23:16:05 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) |
> - (,(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