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

Add burly-default-buffer, fix #30 #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tumashu
Copy link

@tumashu tumashu commented Mar 30, 2021

  • burly.el (burly-default-buffer): New variable.
    (burly--bufferize-window-state): use `burly-default-buffer'.

* burly.el (burly-default-buffer): New variable.
(burly--bufferize-window-state): use `burly-default-buffer'.
@alphapapa
Copy link
Owner

Thanks, but when sending a PR, you should explain the rationale. And when adding a new feature or workaround, as this one seems to, it should generally be discussed in an issue first, because this may not be the best approach to take.

@tumashu
Copy link
Author

tumashu commented Apr 1, 2021

The reason is simple: (burly-url-buffer burly-url) sometime return nil, In this condition, burly will switch to stratch buffer, instead of pop error.

@alphapapa
Copy link
Owner

That would seem like "papering over" the problem of some buffers not being restorable. It would seem preferable to give an error that the user can report so the problem can be fixed properly. Otherwise, users will simply see that Burly does not appear to work properly, restoring a scratch buffer instead of the desired one.

@tumashu
Copy link
Author

tumashu commented Apr 1, 2021

It would seem preferable to give an error that the user can report so the problem can be fixed properly.

I do not think so, give a message better than error, burly can not handle all the cases, fallback to scratch and let user manual handle is very useful.

@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 6303aad to c6d6aae Compare June 21, 2021 08:45
@alphapapa alphapapa added this to the 0.4 milestone Aug 31, 2023
@alphapapa alphapapa self-assigned this Aug 31, 2023
@alphapapa
Copy link
Owner

AFAIK this is solved now, but I'll revisit for 0.4.

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.

2 participants