Skip to content

Comments

Dynamic vertically-adjacent gravity#1371

Merged
cxxxr merged 7 commits intolem-project:mainfrom
BierLiebHaber:dynamic-adjacent-gravity
May 24, 2024
Merged

Dynamic vertically-adjacent gravity#1371
cxxxr merged 7 commits intolem-project:mainfrom
BierLiebHaber:dynamic-adjacent-gravity

Conversation

@BierLiebHaber
Copy link
Contributor

Documentation popups going over the edge of the screen have been driving me mad recently.
This adds a new gravity vertically-adjacent-window-dynamic, that ensures the popup is visible, and sets it as the default gravity for documentation popups.
It also makes the gravity type for documentation popups configurable with *documentation-popup-gravity*

I thought about replacing vertically-adjacent-window but figure some people prefer the old way.

:region-completion))
(in-package :lem-lisp-mode/completion)

(defvar *documentation-popup-gravity* :vertically-adjacent-window-dynamic)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the setting might not be user facing, a docstring would help documentors and users: what does this solve and what are the possible values? 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually like this setting to be user facing, so you are right some documentation is necessary.
Is there a better way to expose user facing variables?
Does lem have something like settings page that I missed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you be fine with this as documentation?

(setf (documentation '*documentation-popup-gravity* 'variable)
      (format nil "Set the gravity of the documentation popup. Possibilities are:~%~s"
              (handler-case (lem/popup-window::ensure-gravity nil)
                (condition (arg) (cdr (type-error-expected-type arg))))))

I don't really want people to have to change the documentation just because they added a new gravity, since it's not really related to the gravities themselves.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, this, really? cxxxr will judge^^

set the gravity

I didn't really follow what this does, so I'm a good guinea ping: what does the gravity solve for the documentation popup?

"Set the gravity of the documentation popup. By default, we ensure the popup is displayed like this to prevent this."

It might enough to give a couple default possibilities and refer to a function to know more.

Future doc generators might want an existing docstring in the defvar. It really depends.


You can pick cxxxr as reviewer when you're ready.

Copy link
Member

@cxxxr cxxxr May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I think it is better to prepare docstrings as static strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. I was mostly intrigued by the possibility of programmatic doc-strings, it does seem like an extremely powerful feature to autogenerate part of the documentation.

I hope the new doc-string is more helpful. There should probably be some documentation for grativies in general. But I'm not sure how it can be presented well to the user, since gravities are created from keyword symbols. One could do something along the lines of: (setf (documentation :my-cool-gravity 'variable) "the best gravity") for all the gravity-keywords, lem would then display the doc-string during completion.

Slightly off topic: Are there any plans for a better config-system? Figuring out what can be set in .lem/config.lisp is somewhat annoying.

@cxxxr
Copy link
Member

cxxxr commented May 24, 2024

I think it's very good!
Thank you!

@cxxxr cxxxr merged commit 8fe59d4 into lem-project:main May 24, 2024
@BierLiebHaber BierLiebHaber deleted the dynamic-adjacent-gravity branch May 24, 2024 13:14
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