commit style redux

commit style redux

[Miles Bader]

Hmm, so since Emacs is going to move to a real source-control system in
the near(ish) future, maybe people could start trying to use a
commit-style which is more friendly to such systems?

Not only is this good practice for the future, but it also creates a
much nicer change history when the CVS repo is converted to the new
system (and even now, for people currently tracking Emacs via other
systems).

E.g., --

 (1) Commit all changed files at once with a single log message (which
     in CVS will result in an identical log message for all committed
     files), not one-by-one.  This is pretty easy using vc-dir now.

 (2) Make the log message describe the entire changeset, perhaps
     including relevant changelog entiries (I often don't bother with
     the latter if it's a trivial sort of change).

     Many modern source-control systems vaguely distinguish the first
     line of the log message to use as a short summary for abbreviated
     history listing (in arch this was explicitly called the summary,
     but many other systems have a similar concept).  So it's nice if
     you can format the log entry like:

        SHORTISH ONE-LINE SUMMARY

        MULTIPLE-LINE DETAILED DESCRIPTION POSSIBLY INCLUDING (OR
        CONSISTING OF) CHANGELOG ENTRIES

     [Even with CVS this style is useful, because web CVS browsing
     interfaces often include the first N words of the log message of
     the most recent commit as a short "most recent change"
     description.]

 (3) Don't phrase log messages assuming the filename is known, because
     in non-file-oriented systems (everything modern other than CVS),
     the log listing tends to be treated as global information, and the
     connection with specific files is less explicit.

     For instance, currently I often see log messages like "Regenerate";
     for modern source-control systems with a global log, it's better to
     have something like "Regenerate configure".

I know I'm always harping on this, but I figure with the change looming
ever closer, I'll try again... :-)

Thanks,

-Miles

-- 
`Life is a boundless sea of bitterness'

Re: commit style redux

[Stefan Monnier]

> Hmm, so since Emacs is going to move to a real source-control system in
> the near(ish) future, maybe people could start trying to use a
> commit-style which is more friendly to such systems?

Yes, please,


        Stefan

Re: commit style redux

[Glenn Morris]

[replying to old message]

Miles Bader wrote:

>  (3) Don't phrase log messages assuming the filename is known, because
>      in non-file-oriented systems (everything modern other than CVS),
>      the log listing tends to be treated as global information, and the
>      connection with specific files is less explicit.

I use M-x log-edit-insert-changelog to auto-populate the vc-log buffer
from my ChangeLog entries. Is this doing the right thing, or does Emacs
need to be changed?

It creates log entries like this:

  * cedet/ede/cpp-root.el (ede-cpp-root-project): Foo

  * srecode/doc-cpp.srt, srecode/doc-default.srt: Bar
 
  * ede.texi (ede-target):
  * org.texi (Refiling notes): Qux

The paths are relative to where each ChangeLog file is, rather than
relative to the root of the project. If there are two files with the
same basename in different subdirectories, things might get ambiguous.

Re: commit style redux

[Juanma Barranquero]

On Sat, Jan 16, 2010 at 04:00, Glenn Morris <rgm@gnu.org> wrote:

> The paths are relative to where each ChangeLog file is, rather than
> relative to the root of the project. If there are two files with the
> same basename in different subdirectories, things might get ambiguous.

There aren't that many matching filenames, and in most cases it should
be obvious from the change which file it is affecting, I think.

BTW, perhaps we should clean the dust from Miles message and use it as
the start of a "commit policy" document.

For example, the point about the SHORTISH ONE-LINE SUMMARY should be
stressed IMHO; it is really quite useful when browsing commits.

Another thing we still do quite often is mixing trivial, unrelated
fixes in a single commit (for example, uncountable times I've fixed
typos, etc. in a ChangeLog at the same time that I was adding an entry
to that ChangeLog). We should try to get used to separate these kinds
of changes even if it means a bit more work (alas, that's much easier
with interactive cherry-picking tools, like git add -i, which I think
Bazaar lacks).

    Juanma

Re: commit style redux

[Thien-Thi Nguyen]

() Glenn Morris <rgm@gnu.org>
() Fri, 15 Jan 2010 22:00:09 -0500

   I use M-x log-edit-insert-changelog to auto-populate the vc-log
   buffer from my ChangeLog entries. Is this doing the right
   thing, or does Emacs need to be changed?

   It creates log entries like this:

     * cedet/ede/cpp-root.el (ede-cpp-root-project): Foo

     * srecode/doc-cpp.srt, srecode/doc-default.srt: Bar

     * ede.texi (ede-target):
     * org.texi (Refiling notes): Qux

   The paths are relative to where each ChangeLog file is, rather
   than relative to the root of the project. If there are two
   files with the same basename in different subdirectories,
   things might get ambiguous.

FWIW, the following substandard (undocumented except for this
message, works for me on 23.0.60.9, tuned to my tastes) code
disambiguates the ChangeLog greps.  It basically:

- looks at each named ChangeLog in the "git status" output;
- inserts the first entry, stripping/unifying the "subject line"
  (sanity-checking that they are all string=, in the process);
- modifies the entries' "* foo" to contain the ChangeLog's
  directory parent prefix (if any) from the project root, so that
  "abc/def/ChangeLog" entry "* foo (bar): baz" ends up looking
  like "* abc/def/foo (bar): baz".

The OLDGIT cruft is for Git 1.5.3.5, which i have transitioned
away from in the last few months and can probably be safely
ignored.

With this, i do "git commit" (either directly via M-x compile) or
through some VC keybinding, `C-c C-d' to populate the buffer, and
`C-c C-c' to finish.

Doubtless a SMOP can get this adapted to (current) Emacs (current)
VC (current) Git with support for other DVCSes as well.

thi

________________________________________________________________________
;;; this is arguably the wrong way to go about things...

(defun commit-log-from-git-status (relative-file-names)
  (let ((changelogs (or (remove-if-not (lambda (s)
                                         (string-match "ChangeLog$" s))
                                       relative-file-names)
                        (error "No ChangeLog files")))
        head body)
    (dolist (fn changelogs)
      (with-temp-buffer
        (insert-file-contents fn)
        (search-forward "\n\n" nil nil 2)
        (push (buffer-substring (point-min) (point)) head)
        (unless (or (null (cdr head))
                    (string= (car head) (cadr head)))
          (error "head mismatch for %s" fn))
        (push (buffer-substring
               (point) (progn (re-search-forward change-log-start-entry-re)
                              (match-beginning 0)))
              body)))
    (setq head (car head)
          head (and (string-match "\n\n\t" head)
                    (substring head (match-end 0)))
          body (map 'list
                    (lambda (fn bod)
                      (let ((pre (and (string-match "ChangeLog$" fn)
                                      (substring fn 0 (match-beginning 0)))))
                        (with-temp-buffer
                          (insert bod)
                          (goto-char (point-min))
                          (while (search-forward "\t* " nil 1)
                            (insert pre))
                          (while (re-search-backward "^\t" nil t)
                            (delete-char 1))
                          (buffer-string))))
                    (nreverse changelogs) body))
    (apply 'concat head body)))

(defun scan-git-status-and-insert-commit-log ()
  (interactive)
  (let ((oldgitp (equal "1" (getenv "OLDGIT")))
        (default-directory (expand-file-name ".."))
        (max (save-excursion
               (if (search-forward "Changed but not updated" nil t)
                   (match-beginning 0)
                 (point-max))))
        sub acc)
    (unless oldgitp
      (let ((root default-directory))
        (find-if (lambda (buf)
                   (with-current-buffer buf
                     (when (and (eq 'compilation-mode major-mode)
                                (eq 'run (process-status buf))
                                (setq sub (expand-file-name default-directory))
                                (search root sub))
                       (setq sub (substring sub (1+ (mismatch root sub)))))))
                 (buffer-list))))
    (save-excursion
      (while (re-search-forward "\tmodified: +\\(.+$\\)" max t)
        (push (concat sub (match-string 1)) acc)))
    (insert (commit-log-from-git-status (nreverse acc)))))

(defvar commit-editmsg-map
  (let ((m (copy-keymap text-mode-map)))
    (define-keys m
      "\C-c\C-d" 'scan-git-status-and-insert-commit-log
      "\C-c\C-c" 'server-edit)
    m))

(defun commit-editmsg-setup ()
  (when (string-match ".git/COMMIT_EDITMSG" (or buffer-file-name ""))
    (set-buffer-file-coding-system 'utf-8)
    (use-local-map commit-editmsg-map)))

(add-hook 'server-visit-hook 'commit-editmsg-setup)

;;; vc-git-hacking.el ends here

Re: commit style redux

[Glenn Morris]

Should I be setting log-edit-strip-single-file-name to nil when using
log-edit-insert-changelog to generate commit logs from ChangeLogs
these days?

If so, it should probably be in Emacs's .dir-locals.el.

Re: commit style redux

[Stefan Monnier]

> Should I be setting log-edit-strip-single-file-name to nil when using
> log-edit-insert-changelog to generate commit logs from ChangeLogs
> these days?

Yes.

> If so, it should probably be in Emacs's .dir-locals.el.

Agreed,


        Stefan

log-edit-strip-single-file-name [was Re: commit style redux]

[Glenn Morris]

Stefan Monnier wrote:

>> Should I be setting log-edit-strip-single-file-name to nil when using
[...]
>> If so, it should probably be in Emacs's .dir-locals.el.
>
> Agreed,

It doesn't work. I guess dir-locals doesn't affect buffers not
visiting files.

Perhaps log-edit-insert-changelog could respect the value of
log-edit-strip-single-file-name in the ChangeLog buffer rather than in
the *VC-log* buffer.

Re: log-edit-strip-single-file-name

[Stefan Monnier]

>>> Should I be setting log-edit-strip-single-file-name to nil when using
> [...]
>>> If so, it should probably be in Emacs's .dir-locals.el.
>> Agreed,
> It doesn't work.  I guess dir-locals doesn't affect buffers not
> visiting files.

This is a "general" problem that we may want to solve: non-file (nor
dired) buffers that are (loosely) associated with a particular
directory should probably obey .dir-locals.el.

> Perhaps log-edit-insert-changelog could respect the value of
> log-edit-strip-single-file-name in the ChangeLog buffer rather than in
> the *VC-log* buffer.

That's an idea.  Another is to change the default value of
log-edit-strip-single-file-name ;-)


        Stefan

Re: log-edit-strip-single-file-name

[Stefan Monnier]

> That's an idea.  Another is to change the default value of
> log-edit-strip-single-file-name ;-)

Or to make this value depend on the backend (that might actually be the
best option),


        Stefan

Re: log-edit-strip-single-file-name

[Dan Nicolaescu]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > > Perhaps log-edit-insert-changelog could respect the value of
  > > log-edit-strip-single-file-name in the ChangeLog buffer rather than in
  > > the *VC-log* buffer.
  > 
  > That's an idea.  Another is to change the default value of
  > log-edit-strip-single-file-name ;-)

Please do.  The current value might have been helpful when commits where
done to a single file at a time, so the file name was redundant.
That's not the case anymore for the vast majority of users.

Re: log-edit-strip-single-file-name

[Dan Nicolaescu]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > >>> Should I be setting log-edit-strip-single-file-name to nil when using
  > > [...]
  > >>> If so, it should probably be in Emacs's .dir-locals.el.
  > >> Agreed,
  > > It doesn't work.  I guess dir-locals doesn't affect buffers not
  > > visiting files.
  > 
  > This is a "general" problem that we may want to solve: non-file (nor
  > dired) buffers that are (loosely) associated with a particular
  > directory should probably obey .dir-locals.el.

How about solvin specific instances of this problem by calling
`hack-dir-local-variables' from `log-edit-mode' and other VC related
modes?



 

Re: log-edit-strip-single-file-name

[Stefan Monnier]

>> >>> Should I be setting log-edit-strip-single-file-name to nil when using
>> > [...]
>> >>> If so, it should probably be in Emacs's .dir-locals.el.
>> >> Agreed,
>> > It doesn't work.  I guess dir-locals doesn't affect buffers not
>> > visiting files.
>> 
>> This is a "general" problem that we may want to solve: non-file (nor
>> dired) buffers that are (loosely) associated with a particular
>> directory should probably obey .dir-locals.el.

> How about solvin specific instances of this problem by calling
> `hack-dir-local-variables' from `log-edit-mode' and other VC related
> modes?

Doesn't sound too bad.  Just try and make sure the log-edit part of the
code is as clean as possible (i.e. modify hack-dir-local-variables or
provide a wrapper or a different function if extra work is needed), so
we have a clear path to add that functionality on other codes
(e.g. vc-dir, diff-mode, ...).


        Stefan

Re: log-edit-strip-single-file-name

[Juri Linkov]

>> It doesn't work.  I guess dir-locals doesn't affect buffers not
>> visiting files.
>
> This is a "general" problem that we may want to solve: non-file (nor
> dired) buffers that are (loosely) associated with a particular
> directory should probably obey .dir-locals.el.

Does this mean that a buffer should be associated with .dir-locals.el
when its `default-directory' is a subdirectory of `default-directory'
of .dir-locals.el?

Then what about file local variables (in the first line or a local
variables list)?  Should a non-file buffer obey them?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: log-edit-strip-single-file-name

[Stefan Monnier]

> Does this mean that a buffer should be associated with .dir-locals.el
> when its `default-directory' is a subdirectory of `default-directory'
> of .dir-locals.el?

Not for all buffers, and clearly not dynamically.  E.g. not for *shell*.


        Stefan

Re: log-edit-strip-single-file-name

[Dan Nicolaescu]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > >> >>> Should I be setting log-edit-strip-single-file-name to nil when using
  > >> > [...]
  > >> >>> If so, it should probably be in Emacs's .dir-locals.el.
  > >> >> Agreed,
  > >> > It doesn't work.  I guess dir-locals doesn't affect buffers not
  > >> > visiting files.
  > >> 
  > >> This is a "general" problem that we may want to solve: non-file (nor
  > >> dired) buffers that are (loosely) associated with a particular
  > >> directory should probably obey .dir-locals.el.
  > 
  > > How about solvin specific instances of this problem by calling
  > > `hack-dir-local-variables' from `log-edit-mode' and other VC related
  > > modes?
  > 
  > Doesn't sound too bad.  Just try and make sure the log-edit part of the
  > code is as clean as possible (i.e. modify hack-dir-local-variables or
  > provide a wrapper or a different function if extra work is needed), so
  > we have a clear path to add that functionality on other codes
  > (e.g. vc-dir, diff-mode, ...).

Here's the first cut, that amazingly works.
log-edit-strip-single-file-name is set correctly from .dir-locals.el
after this patch.
(the code  that sets the local variables  in
hack-dir-local-variables-non-file-buffer should be put in a function
that is also called from hack-local-variables).

WDYT?


--- files.el.~1.1104.~	2009-12-17 08:08:20.000000000 -0800
+++ files.el	2010-02-03 23:38:35.000000000 -0800
@@ -3403,20 +3403,21 @@ is found.  Returns the new class name."
 				      (nth 5 (file-attributes file)))
       class-name)))
 
-(defun hack-dir-local-variables ()
-  "Read per-directory local variables for the current buffer.
+(defun hack-dir-local-variables (&optional dname)
+  "Read per-directory local variables.
+If DNAME is set look in that directory, if not look current buffer's default directory.
 Store the directory-local variables in `dir-local-variables-alist'
 and `file-local-variables-alist', without applying them."
   (when (and enable-local-variables
-	     (buffer-file-name)
-	     (not (file-remote-p (buffer-file-name))))
+	     (or dname (buffer-file-name))
+	     (not (file-remote-p (or dname (buffer-file-name)))))
     ;; Find the variables file.
-    (let ((variables-file (dir-locals-find-file (buffer-file-name)))
+    (let ((variables-file (dir-locals-find-file (or dname (buffer-file-name))))
 	  (class nil)
 	  (dir-name nil))
       (cond
        ((stringp variables-file)
-	(setq dir-name (file-name-directory (buffer-file-name)))
+	(setq dir-name (or dname (file-name-directory (buffer-file-name))))
 	(setq class (dir-locals-read-from-file variables-file)))
        ((consp variables-file)
 	(setq dir-name (nth 0 variables-file))
@@ -3433,6 +3434,18 @@ and `file-local-variables-alist', withou
 	      (push elt dir-local-variables-alist))
 	    (hack-local-variables-filter variables dir-name)))))))
 
+
+(defun hack-dir-local-variables-non-file-buffer (dname)
+  (hack-dir-local-variables dname)
+  (when file-local-variables-alist
+    ;; Any 'evals must run in the Right sequence.
+    (setq file-local-variables-alist
+	  (nreverse file-local-variables-alist))
+    (run-hooks 'before-hack-local-variables-hook)
+    (dolist (elt file-local-variables-alist)
+      (hack-one-local-variable (car elt) (cdr elt))))
+  (run-hooks 'hack-local-variables-hook))
+
 \f
 (defcustom change-major-mode-with-file-name t
   "Non-nil means \\[write-file] should set the major mode from the file name.

--- log-edit.el.~1.56.~	2009-10-03 03:20:50.000000000 -0700
+++ log-edit.el	2010-02-03 23:41:20.000000000 -0800
@@ -152,6 +152,8 @@ can be obtained from `log-edit-files'."
   "If non-nil, remove file name from single-file log entries."
   :type 'boolean)
 
+(put 'log-edit-strip-single-file-name 'safe-local-variable 'booleanp)
+
 (defvar cvs-changelog-full-paragraphs t)
 (make-obsolete-variable 'cvs-changelog-full-paragraphs
                         'log-edit-changelog-full-paragraphs
@@ -365,7 +389,8 @@ commands (under C-x v for VC, for exampl
 \\{log-edit-mode-map}"
   (set (make-local-variable 'font-lock-defaults)
        '(log-edit-font-lock-keywords t))
-  (make-local-variable 'log-edit-comment-ring-index))
+  (make-local-variable 'log-edit-comment-ring-index)
+  (hack-dir-local-variables-non-file-buffer default-directory))
 
 (defun log-edit-hide-buf (&optional buf where)
   (when (setq buf (get-buffer (or buf log-edit-files-buf)))

Re: log-edit-strip-single-file-name

[Stefan Monnier]

> (the code  that sets the local variables  in
> hack-dir-local-variables-non-file-buffer should be put in a function
> that is also called from hack-local-variables).

Yes, that would make sense.

> -    (let ((variables-file (dir-locals-find-file (buffer-file-name)))
> +    (let ((variables-file (dir-locals-find-file (or dname (buffer-file-name))))

Why (or dname (buffer-file-name)) rather than (or buffer-file-name default-directory)?


        Stefan

Re: log-edit-strip-single-file-name

[Dan Nicolaescu]

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

  > > (the code  that sets the local variables  in
  > > hack-dir-local-variables-non-file-buffer should be put in a function
  > > that is also called from hack-local-variables).
  > 
  > Yes, that would make sense.
  > 
  > > -    (let ((variables-file (dir-locals-find-file (buffer-file-name)))
  > > +    (let ((variables-file (dir-locals-find-file (or dname (buffer-file-name))))
  > 
  > Why (or dname (buffer-file-name)) rather than (or buffer-file-name default-directory)?

It looked clearer to me.

Can this patch go in?

Re: log-edit-strip-single-file-name

[Stefan Monnier]

>> > -    (let ((variables-file (dir-locals-find-file (buffer-file-name)))
>> > +    (let ((variables-file (dir-locals-find-file (or dname (buffer-file-name))))
>> Why (or dname (buffer-file-name)) rather than (or buffer-file-name default-directory)?
> It looked clearer to me.

I think that using something else than default-directory would be
a serious source of surprise for the user.

> Can this patch go in?

Not for 23.2, no.


        Stefan

Re: log-edit-strip-single-file-name

[Dan Nicolaescu]

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

  > >> > -    (let ((variables-file (dir-locals-find-file (buffer-file-name)))
  > >> > +    (let ((variables-file (dir-locals-find-file (or dname (buffer-file-name))))
  > >> Why (or dname (buffer-file-name)) rather than (or buffer-file-name default-directory)?
  > > It looked clearer to me.
  > 
  > I think that using something else than default-directory would be
  > a serious source of surprise for the user.

That would not happen, the default-directory is passed to
hack-local-directory-variable as an optional argument. 
But if you prefer to use default-directory instead of passing it as an
argument, I'll do that.

Re: log-edit-strip-single-file-name

[Stefan Monnier]

>> I think that using something else than default-directory would be
>> a serious source of surprise for the user.
> That would not happen, the default-directory is passed to
> hack-local-directory-variable as an optional argument. 

Right, but providing it as an argument is implicitly indicating that it
could be something else in some cases.

> But if you prefer to use default-directory instead of passing it as an
> argument, I'll do that.

Yes, that'd be better.


        Stefan