Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update code to show magit buffers in full screen #3319

Closed
wants to merge 1 commit into from

Conversation

bmag
Copy link
Collaborator

@bmag bmag commented Oct 9, 2015

A recent change in Magit removed magit-status-buffer-switch-function and introduced magit-display-buffer-function, causing our configuration for full screen Magit buffers to break. This PR updates the relevant configuration.
There is some discussion here

I don't remember if the old configuration opened all magit-mode derived buffers (e.g. including log buffers and diff buffers) in full screen or just the status buffer. The new configuration affects all magit-mode derived buffers.

cc @d12frosted

@d12frosted
Copy link
Collaborator

Hey! Thanks for such a fast PR.

I think all buffers were full-screen. Well, except when you press return on commit - it is shown in split buffer.

@bmag
Copy link
Collaborator Author

bmag commented Oct 9, 2015

when you press return on commit - it is shown in split buffer.

With my PR, the commit is shown full screen too. I think I know why, but lets see if the discussion in Magit will provide a better solution.

@bmag bmag changed the title Update code to show magit buffers in full screen [WIP] Update code to show magit buffers in full screen Oct 9, 2015
@d12frosted
Copy link
Collaborator

Well, I am ok with both, actually. So yeah, let's hear what others think.

kostya-sh referenced this pull request in magit/magit Oct 13, 2015
Use low-level `display-buffer' instead of high-level buffer switching
functions.  In combination with other changes described below, this
gives users complete control over how buffers are to be displayed,
without having to fight against hard-coded defaults.

Instead of hard-coding the rules that govern how windows are selected to
display certain buffers, define them in one place.  The existing rules
are collected in the new function `magit-display-buffer-traditional'.

That function is the default value of the new option
`magit-display-buffer-function', which specifies the function used by
`magit-display-buffer' to actually display the buffer.

All Magit buffers are displayed using `magit-display-buffer', which is
called by `magit-mode-setup' and `magit-process-buffer'.  Together these
two functions are responsible for the creation of each and every Magit
buffer.  `magit-display-buffer' should never called directly by anything
else. (Popup buffers are not Magit buffers, their major-mode does not
derived from `magit-mode'.)

Unlike `display-buffer', `magit-display-buffer' also selects the window,
unless the new variable `magit-display-buffer-noselect' is let-bound to
a non-nil value.  This variable is only intended for special cases.

Two options, and therefore quick ways of customizing the behavior, fall
victim to this change: `magit-status-buffer-switch-function' and
`magit-diff-switch-buffer-function'.  We cannot preserve these options
and use them in `magit-display-buffer-traditional' because they were
used to specify arbitrary high-level buffer switching functions, which
we cannot simply map to low-level display actions.
@d12frosted
Copy link
Collaborator

Bump. Any update on this?

@bmag
Copy link
Collaborator Author

bmag commented Oct 15, 2015

I was waiting to see if anyone else had something to say in the other discussion, but it doesn't seem so. I'll work on an improved solution sometime before Sunday.

@d12frosted
Copy link
Collaborator

Ah cool. Thanks for the heads up.

@bmag bmag changed the title [WIP] Update code to show magit buffers in full screen Update code to show magit buffers in full screen Oct 17, 2015
@bmag
Copy link
Collaborator Author

bmag commented Oct 17, 2015

@d12frosted this PR is ready now

@d12frosted
Copy link
Collaborator

Sweet. Thanks for the update.

@TheBB
Copy link
Collaborator

TheBB commented Oct 31, 2015

Thanks! Cherry-picked in develop. You can safely delete your branch.

@TheBB TheBB closed this Oct 31, 2015
@bmag bmag deleted the update-magit-fullscreen branch October 31, 2015 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants