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

(#37) feature: selecting a sensible terminal, configurable terminal command, and notifying when we can't open it #40

Merged
merged 3 commits into from
Feb 26, 2024

Conversation

mattkae
Copy link
Owner

@mattkae mattkae commented Feb 25, 2024

What's new?

  • We now select a sensible terminal if you've fully installed the snap or the binary (e.g. sudo make install). The sensible terminal command needs to be found in your path
  • Alternatively, you may specify a terminal command via the terminal: MY_COMMAND_STRING key in the config file
  • If we fail to launch your terminal, we'll notify you via libnotify

Copy link
Contributor

@OPNA2608 OPNA2608 left a comment

Choose a reason for hiding this comment

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

Tested by applying onto 0.1.0. Terminal command works now, as does overriding it with a command in the config. Haven't checked the notifying part.

Just one suggestion about install location, now that the GNUInstallDirs PR was merged.

CMakeLists.txt Outdated Show resolved Hide resolved
Co-authored-by: Cosima Neidahl <opna2608@protonmail.com>
@mattkae
Copy link
Owner Author

mattkae commented Feb 25, 2024

Haven't checked the notifying part.

I'm just using libnotify there, so I imagine it's present almost everywhere. LMK if you have any info to think otherwise

Comment on lines 17 to 24
char buffer[512];

snprintf(
buffer,
sizeof buffer,
"command -v %s > /dev/null 2>&1",
name
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know if it would make sense to ensure that name length doesn't exceed however much space is left in buffer after everything else, otherwise a long name could cut off parts (or the entirety) of > /dev/null 2>&1.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sounds good to me!

@mattkae mattkae changed the title feature: selecting a sensible terminal, configurable terminal command, and notifying when we can't open it feature: (#37) selecting a sensible terminal, configurable terminal command, and notifying when we can't open it Feb 26, 2024
@mattkae mattkae changed the title feature: (#37) selecting a sensible terminal, configurable terminal command, and notifying when we can't open it (#37) feature: selecting a sensible terminal, configurable terminal command, and notifying when we can't open it Feb 26, 2024
@mattkae mattkae merged commit 0267280 into release/0.1.1 Feb 26, 2024
1 check passed
@mattkae mattkae deleted the feature/sensible-terminal branch February 26, 2024 18:49
Conan-Kudo pushed a commit to Conan-Kudo/miracle-wm that referenced this pull request Apr 23, 2024
…minal command, and notifying when we can't open it (mattkae#40)

* feature: selecting a sensible terminal, configurable terminal command, and notifying when we can't open it

* bugfix: using CMAKE_INSTALL_BINDIR for  miracle-wm-sensible-terminal

Co-authored-by: Cosima Neidahl <opna2608@protonmail.com>

* enhancement: update program_exists check wihout bounds check

---------

Co-authored-by: Cosima Neidahl <opna2608@protonmail.com>
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.

None yet

2 participants