[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#12537: Acknowledgement (support for git commit --amend/--signoff)
From: |
Dmitry Gutov |
Subject: |
bug#12537: Acknowledgement (support for git commit --amend/--signoff) |
Date: |
Mon, 01 Oct 2012 07:59:15 +0400 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20120907 Thunderbird/15.0.1 |
On 01.10.2012 7:16, Stefan Monnier wrote:
- (,(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:]-].
Ok. How about I also add the limitation that the first character must be
a capital letter? message-font-lock-keywords has that.
+(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"?
Works for me.
+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")))
Okay. That's definitely less awkward than my proposed change.
+(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?
Sure, will do. I like the "Log-Edit/git" option better.
+ "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.
I think we're entering the feature freeze period right about now. Is it
okay if I install the updated patch 16 hours or so later?