Commit b68359f9 authored by Jonas Bernoulli's avatar Jonas Bernoulli

don't use section bindings from other buffers in process buffer

Ultimately all sections are inserted using `magit-insert-section',
which binds a few variables dynamically, to pass information to nested
invocations, among other things so that those invocations know what to
use as the parent section of the section that they are inserting.

Normally I avoid the use of dynamic bindings to pass around information,
but in this case it seems appropriate, because the "nested"
`magit-insert-section' invocations often cannot be found lexically
inside the outer invocation, and the cost of passing these values around
as arguments would therefore be very high; a great many functions (which
shouldn't have to concern themselves with this particular implementation
detail) would need additional arguments.

Also the current approach works quite well, because normally the process
of inserting a section and its subsections is fully completed before
other, unrelated sections are inserted into another buffer.

However there is an exception which wasn't handled properly.  While
inserting sections in some buffer, it can become necessary to insert a
section into the process buffer.  This would for example be the case if
`magit-git-debug' is set to `t', and some section inserter runs git to
get a value but gets a non-zero exit status instead.  That normally is
not an error from Magit's perspective, but when `magit-git-debug' is
non-nil, then the error message is logged to the process buffer anyway.

In cases like this, outer bindings for `magit-section-insert--parent'
and `magit-section-insert--oldroot' are still in effect but we failed to
determine whether they are valid, which they are not because they are
about another section being inserted into another buffer.

Because process buffers are never refreshed (the refresh commands just
silently forgo doing so), this can be fixed easily, by binding these
variables to the appropriate values (`nil' or the buffer-local value of
`magit-root-section') around all calls to `magit-insert-section' which
insert a section into the process buffer.

We do not have to do the same for `magit-insert-section--current'
because the value is not used by `magit-insert-section'.  This variable
might be used in the BODY of the macro, but by the time that happens the
macro has already bound the variable to the correct value, overriding
and ignoring all bindings that might be in effect outside of the BODY.

Fixes #2669.
parent 99c10acb
......@@ -27,5 +27,9 @@ Fixes since v2.7.0
* magit-popup-describe-function could create an extra window on wide
* Sections in the process buffer sometimes had invalid parent
sections, causing movement and visibility commands to fail, and
expected Git errors not to be handled gracefully in some cases.
......@@ -227,7 +227,9 @@ optional NODISPLAY is non-nil also display it."
(when magit-process-log-max
(let ((inhibit-read-only t))
(let ((inhibit-read-only t)
(magit-insert-section--parent nil)
(magit-insert-section--oldroot nil))
(make-local-variable 'text-property-default-nonsticky)
(magit-insert-section (processbuf)
(insert "\n")))))
......@@ -506,7 +508,8 @@ Magit status buffer."
(defun magit-process-insert-section (pwd program args &optional errcode errlog)
(let ((inhibit-read-only t)
(magit-insert-section--parent magit-root-section))
(magit-insert-section--parent magit-root-section)
(magit-insert-section--oldroot nil))
(goto-char (1- (point-max)))
(magit-insert-section (process)
(insert (if errcode
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment