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

feat: customize a blacklist of mode where topspace should not be activated #3

Closed
danilevy1212 opened this issue Mar 2, 2022 · 7 comments · Fixed by #4
Closed

feat: customize a blacklist of mode where topspace should not be activated #3

danilevy1212 opened this issue Mar 2, 2022 · 7 comments · Fixed by #4
Assignees
Labels
enhancement New feature or request

Comments

@danilevy1212
Copy link

Is your feature request related to a problem? Please describe.

Some modes, like telega, org-agenda-mode look really bad with this mode active, which prevents me from using the global-topspace-mode

Describe the solution you'd like
It would be nice if there was a defcustom variable to customize certain modes where the user does not wish this mode to be active.

@trevorpogue
Copy link
Owner

trevorpogue commented Mar 2, 2022

Hi, thanks for the input! This sounds like a good idea and I can implement this. However, first I have some questions for you:

  • Is this issue fixed if you set topspace-autocenter-buffers to nil? Do this by adding this to your config file then restarting Emacs: (custom-set-variables '(topspace-autocenter-buffers nil)). I would expect everything to look the same whether topspace-mode is enabled or not in any given buffer when that variable is set to nil, unless you scroll above the top line in that buffer (but please prove me wrong if this is not the case).

  • Could you provide a screenshot of what looks bad in these modes? I am trying out org-agenda-mode with topspace-mode but it's acting as expected from my end (and having issues getting telega installed).

  • What Emacs version are you using? (use C-h C-a to see it)

I think it will still be a good idea to implement the feature you mentioned. I propose making a customizable variable called topspace-enable-from-global-p-function (or something along those lines) for this that can be set to a user function, and the user function can return non-nil or nil to signal that topspace-mode should be enabled or not from global-topspace-mode in that buffer, respectively. Does that sound good to you?

@trevorpogue trevorpogue self-assigned this Mar 2, 2022
@trevorpogue trevorpogue added the enhancement New feature or request label Mar 2, 2022
@danilevy1212
Copy link
Author

Hey @trevorpogue! I'll answer in order:

  • The issue does go away and exactly like you said, it looks topspace-mode wasn't turned on, except being able to move the visual lines beyond the first line.
  • Sure! Here it goes :) For telega:
    image
    For org-(super)-agenda:
    image
  • I am using doom emacs, emacs-version: -> GNU Emacs 28.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.31, cairo version 1.16.0)

The solution you propose sounds very very flexible, I like a lot :)

@trevorpogue
Copy link
Owner

Okay thanks for the information, it's helpful. I'm working on the feature. In the meantime, I was curious, do you like the overall behaviour when topspace-autocenter-buffers is on (set to t), or do you like it better when it's turned off (set to nil)? I'm just trying to figure out if people would prefer it to be enabled or not by default.

@danilevy1212
Copy link
Author

Definitely on by default for me. Honestly, without it on i would just forget it's there.

@trevorpogue
Copy link
Owner

Okay that's good, I have the same feeling.

trevorpogue added a commit that referenced this issue Mar 4, 2022
Resolves #3.
* Add customizable variable `topspace-active`  to determine when `topspace-mode` mode is active / has any effect on buffer.
It can be set to t, nil, or a predicate function. This is useful in particular when `global-topspace-mode` is enabled but you want
`topspace-mode` to be inactive in certain buffers or in any specific
circumstance.  When inactive, `topspace-mode` will still technically be on,
but will be effectively off and have no effect on the buffer.

* Allow `topspace-autocenter-buffers` to be either t, nil, or a predicate function (before it could only be t or nil)
@trevorpogue
Copy link
Owner

So based on the screenshots its behaving as designed except its pushing the text in the top window a few lines too far down. I've noticed this happens in buffers with little pictures/emoticon type characters in it which make the line height taller than before, but that is a separate issue for some other time.

I've added a variable called topspace-active which can be set to t, nil, or a predicate function. When the variable or function returns t, topspace-mode will act normally as before. When the variable or function returns nil, topspace-mode will technically be on, but effectively will be off and there would be no difference in the buffer compared to if the mode is off.

This is the most flexible way possible to filter the mode being on/off from global-topspace-mode as far as I can tell, because this way the mode can be activated/deactivated in real time based on any custom circumstance. However, with an enable-from-global approach like we talked about earlier, the mode can only be allowed/prevented from being turned on after manually turning on global-topspace-mode or after major modes change, as far as I can tell.

I also made topspace-autocenter-buffers allowed to be a function to let the feature be activated/deactivated based on any custom circumstance.

@danilevy1212
Copy link
Author

Thank you so much! @trevorpogue

For anyone else that is interested, this is how I have it configured now, it works like a charm :)

(use-package! topspace
  :config
  (defun dan/topspace--active-p ()
    (not (memq major-mode '(vterm-mode org-agenda-mode))))
  :custom
  (topspace-active #'dan/topspace--active-p)
  :hook
  (doom-first-buffer . global-topspace-mode))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants