unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
       [not found] <E1VlqjS-0006O3-Fn@vcs.savannah.gnu.org>
@ 2013-11-29  4:39 ` Dmitry Gutov
  2013-11-29 13:33   ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-11-29  4:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

> revno: 115265
>   * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author
> headers.

This looks like a fix of previously-implemented functionality, but I'm
not sure I like the result.

Pros: `log-edit-insert-changelog' and `log-edit-show-files' are nifty.

Cons:

1. The "Author" header will be almost always unused in other projects,
aside from Emacs, I'm contributing to. So it'll stick out like a sore
thumb. And even with Emacs, when the ChangeLog entry has the proper
author mentioned, it's auto-filled anyway.

2. The "Summary" doesn't add anything in the way of features, I can just
as well write it in the first line of the message.

When the buffer is displayed initially, the point is before "Summary:",
so now I have to move it first before I can write the commit message.

`vc-git-log-edit-toggle-amend' is not aware of this header and inserts
the whole message, summary and the rest, below the headers, leaving
"Summary" empty. Although this should be fixable.

3. If I decide I don't want to commit now, and I kill the log-edit
buffer (is there any other way to dismiss it?), the window and buffer
created by `log-edit-show-files', doesn't go anywhere, and I have to
take care of it separately. Also fixable.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-29  4:39 ` trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers Dmitry Gutov
@ 2013-11-29 13:33   ` Stefan Monnier
  2013-11-29 14:53     ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-11-29 13:33 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

> 1. The "Author" header will be almost always unused in other projects,
> aside from Emacs, I'm contributing to. So it'll stick out like a sore
> thumb. And even with Emacs, when the ChangeLog entry has the proper
> author mentioned, it's auto-filled anyway.

The intention was to remind people that they should think about who's
the author.  Indeed, it's particularly useful for us but less so for
many other projects.  Maybe we can set it as a .dir-locals.el customization.

> 2. The "Summary" doesn't add anything in the way of features, I can just
> as well write it in the first line of the message.

The RFC822 format does add features, since there are a few other special
headers (Author, Fixes, Amend, maybe a few more).

> When the buffer is displayed initially, the point is before "Summary:",
> so now I have to move it first before I can write the commit message.

Indeed that's a bug.  If someone can fix that, that's be very welcome.

We should also use some of the message-mode commands to move inside the
header, as was suggested here recently.

> `vc-git-log-edit-toggle-amend' is not aware of this header and inserts
> the whole message, summary and the rest, below the headers, leaving
> "Summary" empty. Although this should be fixable.

Sounds like a bug, indeed.

> 3. If I decide I don't want to commit now, and I kill the log-edit
> buffer (is there any other way to dismiss it?), the window and buffer
> created by `log-edit-show-files', doesn't go anywhere, and I have to
> take care of it separately. Also fixable.

Indeed, that needs to be fixed.  It's easy to fix when we kill the
*vc-log* buffer, but it might be trickier to fix when you just bury-it :-(
Maybe log-edit-show-files shouldn't be in the default log-edit-hook.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-29 13:33   ` Stefan Monnier
@ 2013-11-29 14:53     ` Dmitry Gutov
  2013-11-29 17:04       ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-11-29 14:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 29.11.2013 15:33, Stefan Monnier wrote:
> The intention was to remind people that they should think about who's
> the author.  Indeed, it's particularly useful for us but less so for
> many other projects.

Isn't it too late to remind when the log-edit buffer is open? Normally, 
you create ChangeLog entries first (and they should include the right 
author), only then open the vc-dir buffer, mark the files and then see 
the log-edit buffer.

> Maybe we can set it as a .dir-locals.el customization.

It would be better, but since dir-locals overrides personal 
customizations, it wouldn't be ideal either, as far as I'm concerned.

> The RFC822 format does add features, since there are a few other special
> headers (Author, Fixes, Amend, maybe a few more).

True. But is it relevant to the question of whether to include the 
Summary and Author headers by default?

Another problem with Summary, I believe, is that it's often unused in 
Emacs commit messages.

> We should also use some of the message-mode commands to move inside the
> header, as was suggested here recently.

Not sure which thread, or which commands you mean. But `C-e' works well 
enough for moving after "Summary:".

 > Indeed, that needs to be fixed.  It's easy to fix when we kill the
 > *vc-log* buffer, but it might be trickier to fix when you just
 > bury-it :(

Since bury-buffer doesn't have a default binding in log-edit, I think 
that's not much of a problem. But we can follow Magit's (and 
message-mode's) example and create a command that would do the burying 
the smart way (and bind it to C-c C-k).

 > Maybe log-edit-show-files shouldn't be in the default log-edit-hook.

I'd rather we fix it than hide it. Another approach would be to instead 
of creating a separate buffer and window, output its text at the bottom 
of the log-edit buffer. And either make it a read-only-not-real text, or 
comment it out somehow and remove that section before the commit is made.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-29 14:53     ` Dmitry Gutov
@ 2013-11-29 17:04       ` Stefan Monnier
  2013-11-29 22:37         ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-11-29 17:04 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> The intention was to remind people that they should think about who's
>> the author.  Indeed, it's particularly useful for us but less so for
>> many other projects.
> Isn't it too late to remind when the log-edit buffer is open? Normally, you
> create ChangeLog entries first (and they should include the right author),
> only then open the vc-dir buffer, mark the files and then see the
> log-edit buffer.

I think of this "reminding" as a process rather than a one-time event,
so I'm not too worried about it being "too late".
It's not perfect, but it's better than nothing, I think.

>> Maybe we can set it as a .dir-locals.el customization.
> It would be better, but since dir-locals overrides personal customizations,
> it wouldn't be ideal either, as far as I'm concerned.

Having an extra empty "Author:" field might hurt your aesthetic
sensibility, but I don't see how it could be really harmful, so I think
it's perfectly OK even if it's hard to override (and of course, it can
still be overridden, if you're sufficiently motivated).

>> The RFC822 format does add features, since there are a few other special
>> headers (Author, Fixes, Amend, maybe a few more).
> True.  But is it relevant to the question of whether to include the Summary
> and Author headers by default?

What would you include, instead?

> Another problem with Summary, I believe, is that it's often unused in Emacs
> commit messages.

IIUC that's a problem in Emacs's commit messages, not in vc-log-edit,
and the presence of the "Summary:" header might actually encourage
people to change their habit.

>> We should also use some of the message-mode commands to move inside the
>> header, as was suggested here recently.
> Not sure which thread, or which commands you mean.  But `C-e' works well
> enough for moving after "Summary:".

C-a could move to "right after the :".

>> Indeed, that needs to be fixed.  It's easy to fix when we kill the
>> *vc-log* buffer, but it might be trickier to fix when you just
>> bury-it :(
> Since bury-buffer doesn't have a default binding in log-edit, I think that's
> not much of a problem. But we can follow Magit's (and message-mode's)
> example and create a command that would do the burying the smart way (and
> bind it to C-c C-k).

Sounds good.

>> Maybe log-edit-show-files shouldn't be in the default log-edit-hook.
> I'd rather we fix it than hide it.

OK.

> Another approach would be to instead of creating a separate buffer and
> window, output its text at the bottom of the log-edit buffer.  And
> either make it a read-only-not-real text, or comment it out somehow
> and remove that section before the commit is made.

These could work as well, indeed.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-29 17:04       ` Stefan Monnier
@ 2013-11-29 22:37         ` Dmitry Gutov
  2013-11-30  2:29           ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-11-29 22:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 29.11.2013 19:04, Stefan Monnier wrote:
> I think of this "reminding" as a process rather than a one-time event,
> so I'm not too worried about it being "too late".
> It's not perfect, but it's better than nothing, I think.

If people not mentioning actual authors of contributed patches in 
ChangeLog is a real problem (I'm not sure it is), how about augmenting 
the ChangeLog font-locking instead, to highlight this field more 
prominently, prepend it with ephemeral text "Author:", and/or add help 
echo that would remind them that this field is for the actual author and 
their email.

> Having an extra empty "Author:" field might hurt your aesthetic
> sensibility, but I don't see how it could be really harmful, so I think
> it's perfectly OK even if it's hard to override (and of course, it can
> still be overridden, if you're sufficiently motivated).

Indeed, it's not the end of the world.

>>> The RFC822 format does add features, since there are a few other special
>>> headers (Author, Fixes, Amend, maybe a few more).
>> True.  But is it relevant to the question of whether to include the Summary
>> and Author headers by default?
>
> What would you include, instead?

I don't know. "Fixes:"? :)

Why do we need to include anything at all? If this desire has anything 
to do with the black line that can show up even when there are no actual 
headers above it, I believe we can sufficiently improve 
`log-edit-font-lock-keywords' to only render the line below actual 
headers (or summaries that very much look like header-value, I guess).

> IIUC that's a problem in Emacs's commit messages, not in vc-log-edit,
> and the presence of the "Summary:" header might actually encourage
> people to change their habit.

I've seen many times people include several entirely unrelated changes 
in one commit. How does one write a summary line for them?

If you do think that's a problem, having "Summary:" included in the 
default headers (e.g. be the sole one) might indeed improve that.

As long as point is after ":" when the buffer is created and `C-a' is 
bound to `messge-beginning-of-line', I think the main drawback would be 
that it's different from the other Git commit message edit interfaces I 
know (and people might be used to), namely Vim and Magit.

Both of them implicitly consider the first line to be the summary, and 
they indicate with highlighting when it exceeds 50 characters. In 
log-edit, users might be checking that via the mode-line and 
column-number-mode, but the presence of the header name would mess with 
that.

Adding special, summary-header-targeted highlighting would improve the 
situation.

>> Since bury-buffer doesn't have a default binding in log-edit, I think that's
>> not much of a problem. But we can follow Magit's (and message-mode's)
>> example and create a command that would do the burying the smart way (and
>> bind it to C-c C-k).
>
> Sounds good.

Actually, now that I've checked, `C-c C-k' in both `message-mode' and 
`git-commit-mode' that Magit uses kill the buffer after doing some 
cleanup, not bury it. Would you be fine with that?

>> Another approach would be to instead of creating a separate buffer and
>> window, output its text at the bottom of the log-edit buffer.  And
>> either make it a read-only-not-real text, or comment it out somehow
>> and remove that section before the commit is made.
>
> These could work as well, indeed.

I think it can look better than using a separate window. But then again, 
it's how other editors do it, so it might just be more familiar.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-29 22:37         ` Dmitry Gutov
@ 2013-11-30  2:29           ` Stefan Monnier
  2013-11-30 16:02             ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-11-30  2:29 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> I think of this "reminding" as a process rather than a one-time event,
>> so I'm not too worried about it being "too late".
>> It's not perfect, but it's better than nothing, I think.
> If people not mentioning actual authors of contributed patches in ChangeLog
> is a real problem (I'm not sure it is), how about augmenting the ChangeLog
> font-locking instead, to highlight this field more prominently, prepend it
> with ephemeral text "Author:", and/or add help echo that would remind them
> that this field is for the actual author and their email.

Note that a solution that applies only to ChangeLog wouldn't help for
the `elpa' branch.  And hopefully at some point it won't help for
`emacs' either because we'll have finally rid ourselves of the
ChangeLog files.

> I've seen many times people include several entirely unrelated changes in
> one commit.  How does one write a summary line for them?

That's the problem of the committer.  The notion of "summary" is not
my invention.  It was there in GNU Arch, is there is Bzr, and is probably
present in several other VCSes.

> As long as point is after ":" when the buffer is created and `C-a' is bound
> to `messge-beginning-of-line', I think the main drawback would be that it's
> different from the other Git commit message edit interfaces I know (and
> people might be used to), namely Vim and Magit.

> Both of them implicitly consider the first line to be the summary, and they
> indicate with highlighting when it exceeds 50 characters.

We still also support the "first line is summary", for the sake of grumpy
users, but that doesn't extend naturally to Author, Fixes, Amend, ...

> Adding special, summary-header-targeted highlighting would improve
> the situation.

We could easily highlight Summary specially after the Nth character.
[ Not sure why it should be 50, and that number might need to
  depend on the backend, but other than that, I don't see
  any particular difficulty.  ]

> Actually, now that I've checked, `C-c C-k' in both `message-mode' and
> git-commit-mode' that Magit uses kill the buffer after doing some cleanup,
> not bury it. Would you be fine with that?

Seems a bit pointless since C-x k (and kill-this-buffer) works as well
for *VC-Log*, AFAIK.

> I think it can look better than using a separate window.  But then again,
> it's how other editors do it, so it might just be more familiar.

I used a separate window because it seemed easier, avoiding the usual
problems with display-only-text or with text-that-will-disappear, which
both can lead to surprising behaviors for the unsuspecting user.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-30  2:29           ` Stefan Monnier
@ 2013-11-30 16:02             ` Dmitry Gutov
  2013-12-01 21:10               ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-11-30 16:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 30.11.2013 04:29, Stefan Monnier wrote:
> Note that a solution that applies only to ChangeLog wouldn't help for
> the `elpa' branch.  And hopefully at some point it won't help for
> `emacs' either because we'll have finally rid ourselves of the
> ChangeLog files.

Ok, good point. If other contributors have no new suggestions, I'll be 
content with a user option.

> We could easily highlight Summary specially after the Nth character.
> [ Not sure why it should be 50, and that number might need to
>    depend on the backend, but other than that, I don't see
>    any particular difficulty.  ]

50 is just a convention, so it shouldn't be inherently different for any 
other backend. And a safe-local user option will allow users to set it 
per project.

>> Actually, now that I've checked, `C-c C-k' in both `message-mode' and
>> git-commit-mode' that Magit uses kill the buffer after doing some cleanup,
>> not bury it. Would you be fine with that?
>
> Seems a bit pointless since C-x k (and kill-this-buffer) works as well
> for *VC-Log*, AFAIK.

Why do we have `message-kill-buffer', `rcirc-multiline-minor-cancel' and 
`org-kill-note-or-show-branches'?

As I see it, they mean "abort the current action, clean up its data, 
restore the previous window configuration". Having a similar command 
with the same binding in log-edit would be natural. Among other things, 
it would delete the *log-edit-files* window and quit-window on the 
current buffer (maybe deleting the window as a result).

We can go against the similar commands and refrain from killing the 
buffer, but why keep it? We can also save the aborted commit message to 
log-edit-comment-ring.

>> I think it can look better than using a separate window.  But then again,
>> it's how other editors do it, so it might just be more familiar.
>
> I used a separate window because it seemed easier, avoiding the usual
> problems with display-only-text or with text-that-will-disappear, which
> both can lead to surprising behaviors for the unsuspecting user.

Ok, I can work with either approach.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-11-30 16:02             ` Dmitry Gutov
@ 2013-12-01 21:10               ` Stefan Monnier
  2013-12-02  2:40                 ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-12-01 21:10 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> Seems a bit pointless since C-x k (and kill-this-buffer) works as well
>> for *VC-Log*, AFAIK.
> Why do we have `message-kill-buffer', `rcirc-multiline-minor-cancel' and
> org-kill-note-or-show-branches'?

Can't tell you without looking at them.  Presumably they do more than
just kill the buffer and using a kill-buffer-hook was not an option..
In the case of message-kill-buffer' it seems to be trying to deal with
the fact that the content of the buffer is like that of an unsaved file,
but buffer-file-name is non-nil, so it can't rely on the standard
kill-buffer handling.

In the case of *VC-Log* we could have a kill-buffer-hook which prompts
the user and aborts the kill if the user says he doesn't want to throw
away his incomplete comment.  But as you point out below, we could also
instead have a kill-buffer-hook which saves the incomplete comment to
log-edit-comment-ring so the buffer can be deleted without losing
valuable info and hence without prompting.

> As I see it, they mean "abort the current action, clean up its data, restore
> the previous window configuration". Having a similar command with the same
> binding in log-edit would be natural. Among other things, it would delete
> the *log-edit-files* window and quit-window on the current buffer (maybe
> deleting the window as a result).

We might indeed provide a key to "hide" the *VC-Log* buffer without
killing it.  And that should hide (or kill) the *log-edit-files* as well.

> We can go against the similar commands and refrain from killing the buffer,
> but why keep it? We can also save the aborted commit message to
> log-edit-comment-ring.

As long as the data is stored in log-edit-comment-ring I think it's OK
to kill without prompting.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-01 21:10               ` Stefan Monnier
@ 2013-12-02  2:40                 ` Dmitry Gutov
  2013-12-02 13:53                   ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-12-02  2:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 01.12.2013 23:10, Stefan Monnier wrote:
> In the case of *VC-Log* we could have a kill-buffer-hook which prompts
> the user and aborts the kill if the user says he doesn't want to throw
> away his incomplete comment.  But as you point out below, we could also
> instead have a kill-buffer-hook which saves the incomplete comment to
> log-edit-comment-ring so the buffer can be deleted without losing
> valuable info and hence without prompting.

kill-buffer-hook runs when the user already chose to kill the current 
buffer (i.e. called kill-buffer and picked the current one), so that 
looks wasteful, unless the prompt is triggered by a new command that 
doesn't otherwise ask for any user input.

> As long as the data is stored in log-edit-comment-ring I think it's OK
> to kill without prompting.

Ok, so I went ahead and installed some changes in 115345 that hopefully 
aren't too far from what you imagined.

`log-edit-kill-buffer' does a bunch of different stuff, so I feel its 
presence as a separate command is justified.

I've also tried to put `log-edit-hide-buf' in kill-buffer-hook instead, 
but this way, opening log-edit buffer and then closing it via 
`log-edit-kill-buffer' made adjacent vertical splits (when present) jump 
too far, AFAICT because then `quit-windows-on' is called on the log-edit 
buffer before the log-edit-files window is deleted.

Also see the FIXME in log-edit-hide-buf.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-02  2:40                 ` Dmitry Gutov
@ 2013-12-02 13:53                   ` Stefan Monnier
  2013-12-02 23:02                     ` Dmitry Gutov
  2013-12-03  2:17                     ` Dmitry Gutov
  0 siblings, 2 replies; 17+ messages in thread
From: Stefan Monnier @ 2013-12-02 13:53 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> In the case of *VC-Log* we could have a kill-buffer-hook which prompts
>> the user and aborts the kill if the user says he doesn't want to throw
>> away his incomplete comment.  But as you point out below, we could also
>> instead have a kill-buffer-hook which saves the incomplete comment to
>> log-edit-comment-ring so the buffer can be deleted without losing
>> valuable info and hence without prompting.
> kill-buffer-hook runs when the user already chose to kill the current buffer
> (i.e. called kill-buffer and picked the current one), so that looks
> wasteful, unless the prompt is triggered by a new command that doesn't
> otherwise ask for any user input.

Go to your nearest file buffer and type: SPC C-x k RET
As you can see, you did choose the buffer to kill and yet you get
prompted to confirm you want to kill that buffer despite its
unsaved changes.

I think it's good to try and make the *VC-Log* (and the *mail*) buffers
behave similarly to file buffers in this respect.

>> As long as the data is stored in log-edit-comment-ring I think it's OK
>> to kill without prompting.
> Ok, so I went ahead and installed some changes in 115345 that hopefully
> aren't too far from what you imagined.
> `log-edit-kill-buffer' does a bunch of different stuff, so I feel its
> presence as a separate command is justified.
> I've also tried to put `log-edit-hide-buf' in kill-buffer-hook instead, but
> this way, opening log-edit buffer and then closing it via
> log-edit-kill-buffer' made adjacent vertical splits (when present) jump too
> far, AFAICT because then `quit-windows-on' is called on the log-edit buffer
> before the log-edit-files window is deleted.
> Also see the FIXME in log-edit-hide-buf.

It's probably the case that log-edit-hide-buf needs to be revisited,
since it dates to before the rewrite of display-buffer, where Martin
arranged to better be able to "undo" a display-buffer (via bury-buffer
or quit-window).


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-02 13:53                   ` Stefan Monnier
@ 2013-12-02 23:02                     ` Dmitry Gutov
  2013-12-03  0:56                       ` Leo Liu
  2013-12-03  3:06                       ` Stefan Monnier
  2013-12-03  2:17                     ` Dmitry Gutov
  1 sibling, 2 replies; 17+ messages in thread
From: Dmitry Gutov @ 2013-12-02 23:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 02.12.2013 15:53, Stefan Monnier wrote:
> Go to your nearest file buffer and type: SPC C-x k RET
> As you can see, you did choose the buffer to kill and yet you get
> prompted to confirm you want to kill that buffer despite its
> unsaved changes.

Ah, thanks, now I get it. This issue is not new, so it didn't seem too 
important.

> I think it's good to try and make the *VC-Log* (and the *mail*) buffers
> behave similarly to file buffers in this respect.

I don't see this problem with mail. When I'm in a message-mode buffer 
(say, writing a reply from Gnus), and I press C-x k RET, Emacs warns me 
that the buffer is modified and asks for confirmation.

See the small patch at the bottom for log-edit. It seems to work, but I 
fear it may complicate some scenarios where buffers are killed 
automatically.

> It's probably the case that log-edit-hide-buf needs to be revisited,
> since it dates to before the rewrite of display-buffer, where Martin
> arranged to better be able to "undo" a display-buffer (via bury-buffer
> or quit-window).

Guess so.

=== modified file 'lisp/vc/log-edit.el'
--- lisp/vc/log-edit.el	2013-12-02 22:13:51 +0000
+++ lisp/vc/log-edit.el	2013-12-02 22:54:52 +0000
@@ -476,6 +476,7 @@
    (set (make-local-variable 'font-lock-defaults)
         '(log-edit-font-lock-keywords t))
    (make-local-variable 'log-edit-comment-ring-index)
+  (add-hook 'kill-buffer-hook 'log-edit-kill-buffer-prompt nil t)
    (hack-dir-local-variables-non-file-buffer))

  (defun log-edit-hide-buf (&optional buf where)
@@ -551,6 +552,11 @@
      (quit-windows-on buf)
      (kill-buffer buf)))

+(defun log-edit-kill-buffer-prompt ()
+  (unless (ring-member log-edit-comment-ring (buffer-string))
+    (or (yes-or-no-p "Discard the comment?")
+        (user-error "Aborted"))))
+
  (defun log-edit-files ()
    "Return the list of files that are about to be committed."
    (ignore-errors (funcall log-edit-listfun)))





^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-02 23:02                     ` Dmitry Gutov
@ 2013-12-03  0:56                       ` Leo Liu
  2013-12-03  3:06                       ` Stefan Monnier
  1 sibling, 0 replies; 17+ messages in thread
From: Leo Liu @ 2013-12-03  0:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel

On 2013-12-03 07:02 +0800, Dmitry Gutov wrote:
> +(defun log-edit-kill-buffer-prompt ()
> +  (unless (ring-member log-edit-comment-ring (buffer-string))
> +    (or (yes-or-no-p "Discard the comment?")
> +        (user-error "Aborted"))))
> +

It starts to look like over-thinking.

Leo



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-02 13:53                   ` Stefan Monnier
  2013-12-02 23:02                     ` Dmitry Gutov
@ 2013-12-03  2:17                     ` Dmitry Gutov
  2013-12-03  3:09                       ` Stefan Monnier
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2013-12-03  2:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 02.12.2013 15:53, Stefan Monnier wrote:
> It's probably the case that log-edit-hide-buf needs to be revisited,
> since it dates to before the rewrite of display-buffer, where Martin
> arranged to better be able to "undo" a display-buffer (via bury-buffer
> or quit-window).

The patch below seems to work. It still feels duct tapey, but I haven't 
found a way to make `pop-to-buffer' split one specific window, short of 
writing a new display-buffer-function.


=== modified file 'lisp/vc/log-edit.el'
--- lisp/vc/log-edit.el	2013-12-02 22:13:51 +0000
+++ lisp/vc/log-edit.el	2013-12-03 01:53:23 +0000
@@ -480,13 +480,8 @@

  (defun log-edit-hide-buf (&optional buf where)
    (when (setq buf (get-buffer (or buf log-edit-files-buf)))
-    ;; FIXME: Should use something like `quit-windows-on' here, but
-    ;; that function never deletes this buffer's window because it
-    ;; was created using `cvs-pop-to-buffer-same-frame'.
      (save-selected-window
-      (let ((win (get-buffer-window buf where)))
-        (if win (ignore-errors (delete-window win))))
-      (bury-buffer buf))))
+      (quit-windows-on buf))))

  (defun log-edit-add-new-comment (comment)
    (when (or (ring-empty-p log-edit-comment-ring)

=== modified file 'lisp/vc/pcvs-util.el'
--- lisp/vc/pcvs-util.el	2013-08-05 14:26:57 +0000
+++ lisp/vc/pcvs-util.el	2013-12-03 02:07:30 +0000
@@ -89,7 +89,10 @@
      (or (let ((buf (get-buffer-window buf))) (and buf (select-window 
buf)))
  	(and pop-up-windows
  	     (ignore-errors (select-window (split-window-below)))
-	     (switch-to-buffer buf nil 'force-same-window))
+             (prog1
+                 (switch-to-buffer buf t 'force-same-window)
+               (set-window-prev-buffers nil nil)
+               (display-buffer-record-window 'window nil 
(current-buffer))))
  	(pop-to-buffer (current-buffer)))))

  (defun cvs-bury-buffer (buf &optional mainbuf)




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-02 23:02                     ` Dmitry Gutov
  2013-12-03  0:56                       ` Leo Liu
@ 2013-12-03  3:06                       ` Stefan Monnier
  2013-12-04  0:38                         ` Dmitry Gutov
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-12-03  3:06 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> I think it's good to try and make the *VC-Log* (and the *mail*) buffers
>> behave similarly to file buffers in this respect.
> I don't see this problem with mail. When I'm in a message-mode buffer (say,
> writing a reply from Gnus), and I press C-x k RET, Emacs warns me that the
> buffer is modified and asks for confirmation.

I didn't mean to say that there's a problem with mail.  Nor that we
should prompt for *VC-Log*.  Just that it would be an acceptable behavior.

> See the small patch at the bottom for log-edit.  It seems to work, but I fear
> it may complicate some scenarios where buffers are killed automatically.

As written earlier, I think it's OK to kill without prompting and
simply stash the buffer's contents in the log-comment history.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-03  2:17                     ` Dmitry Gutov
@ 2013-12-03  3:09                       ` Stefan Monnier
  2013-12-04  0:43                         ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2013-12-03  3:09 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> It's probably the case that log-edit-hide-buf needs to be revisited,
>> since it dates to before the rewrite of display-buffer, where Martin
>> arranged to better be able to "undo" a display-buffer (via bury-buffer
>> or quit-window).
> The patch below seems to work. It still feels duct tapey, but I haven't
> found a way to make `pop-to-buffer' split one specific window, short of
> writing a new display-buffer-function.

I'm not sure it's better than what we have, indeed.

I guess TRT is to "write a new display-buffer-function".  But let's wait
until we bump into actual problems, so we know concrete examples to improve.


        Stefan



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-03  3:06                       ` Stefan Monnier
@ 2013-12-04  0:38                         ` Dmitry Gutov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2013-12-04  0:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 03.12.2013 05:06, Stefan Monnier wrote:
> As written earlier, I think it's OK to kill without prompting and
> simply stash the buffer's contents in the log-comment history.

Okay, done.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers.
  2013-12-03  3:09                       ` Stefan Monnier
@ 2013-12-04  0:43                         ` Dmitry Gutov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2013-12-04  0:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 03.12.2013 05:09, Stefan Monnier wrote:
> I'm not sure it's better than what we have, indeed.

I wouldn't be hard to present a scenario where the window displaying the 
files buffer has a non-empty history of previous buffers, and thus 
shouldn't be deleted, at least in principle. But all such scenarios that 
I can imagine are pretty convoluted, and it wouldn't really be much of a 
problem anyway.

> I guess TRT is to "write a new display-buffer-function".  But let's wait
> until we bump into actual problems, so we know concrete examples to improve.

Yes, let's wait for bug reports.



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-12-04  0:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <E1VlqjS-0006O3-Fn@vcs.savannah.gnu.org>
2013-11-29  4:39 ` trunk r115265: * lisp/vc/vc-dispatcher.el (vc-log-edit): Setup the Summary&Author headers Dmitry Gutov
2013-11-29 13:33   ` Stefan Monnier
2013-11-29 14:53     ` Dmitry Gutov
2013-11-29 17:04       ` Stefan Monnier
2013-11-29 22:37         ` Dmitry Gutov
2013-11-30  2:29           ` Stefan Monnier
2013-11-30 16:02             ` Dmitry Gutov
2013-12-01 21:10               ` Stefan Monnier
2013-12-02  2:40                 ` Dmitry Gutov
2013-12-02 13:53                   ` Stefan Monnier
2013-12-02 23:02                     ` Dmitry Gutov
2013-12-03  0:56                       ` Leo Liu
2013-12-03  3:06                       ` Stefan Monnier
2013-12-04  0:38                         ` Dmitry Gutov
2013-12-03  2:17                     ` Dmitry Gutov
2013-12-03  3:09                       ` Stefan Monnier
2013-12-04  0:43                         ` Dmitry Gutov

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).