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.

Sign up for GitHub

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

Jump to bottom

feat: add support for middle position quick terminal #3100

Merged
merged 2 commits into from
Dec 25, 2024
Merged

feat: add support for middle position quick terminal #3100

merged 2 commits into from
Dec 25, 2024

Conversation

dmehala
Copy link
Collaborator

@dmehala dmehala commented Dec 24, 2024 •
edited
Loading

Description

This commit introduces the ability to launch the quick terminal in the middle position.

Screen Recording 2024-12-24 at 14 59 46

Note to reviewer

  • The quick terminal is currently centered. Should we consider adding a top offset to better align with the screenshot in the issue? Should it be configurable?
  • On large monitors, half the visible frame might be excessively large. To mitigate, I am planning to implement Customize initial size for quick terminal #2384 but we should probably agree on a good default maximum width/height in middle position.
  • I also figured out, reloading the configuration does not update the quick terminal configuration. That is also an issue I will try to fix.

Resolves #2494

I agree to relicense my commit to MIT.

@dmehala
feat: add support for middle position in quick terminal
1497e90 
This update introduces the ability to launch the quick terminal
in the middle position.

Resolves #2494
@dmehala dmehala changed the title feat: add support for middle position in quick terminal feat: add support for middle position quick terminal Dec 24, 2024
@jparise
Copy link
Collaborator

jparise commented Dec 24, 2024 •
edited
Loading

Just a note that tools like Rectangle call this the "center" position. ("Middle" seems to be more about the midpoint of a single axis.)

No strong feelings on this. Just a data point.

mitchellh
mitchellh requested changes Dec 24, 2024
View reviewed changes
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Thanks this looks great. I requested a couple small changes, but I think for an initial go this is fine. Your size issues are noted, I think we can continue to improve and iterate on those post-merge.

src/config/Config.zig Outdated
@@ -5257,6 +5257,7 @@ pub const QuickTerminalPosition = enum {
bottom,
left,
right,
middle,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's rename this to center Ala Rectangles and other tiling WMs.

Please also add docs to the quick-terminal-position config in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the review. Addressed in 44459e9

@mitchellh mitchellh merged commit fddedd7 into ghostty-org:main Dec 25, 2024
17 checks passed
@mitchellh mitchellh deleted the dmehala/quick-terminal-middle branch December 25, 2024 04:25
@lucymhdavies lucymhdavies mentioned this pull request Dec 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mitchellh mitchellh mitchellh requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

quick-terminal-position: add middle for floating middle position
3 participants
@dmehala @jparise @mitchellh