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

Update aerospace extension #14886

Closed
wants to merge 3 commits into from
Closed

Conversation

thewinger
Copy link
Contributor

@thewinger thewinger commented Oct 11, 2024

Description

Adds a new command to change apps of the current workspace.

Screencast

CleanShot 2024-10-11 at 20 05 16

Checklist

- Adds functionality to change apps in current workspace
- Initial commit
@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: aerospace Issues related to the aerospace extension labels Oct 11, 2024
@raycastbot
Copy link
Collaborator

raycastbot commented Oct 11, 2024

Thank you for your contribution! 🎉

🔔 @limonkufu @AmmarCodes you might want to have a look.

You can use this guide to learn how to check out the Pull Request locally in order to test it.

Due to our current reduced availability, the initial review may take up to 10-15 business days.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this file was included in the commit, I think it should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Raycast extension guidelines: “ Please use npm for installing dependencies and include package-lock.json in your pull request.”

Copy link
Contributor

Choose a reason for hiding this comment

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

I should have included it myself, thanks

extensions/aerospace/CHANGELOG.md Outdated Show resolved Hide resolved
@AmmarCodes
Copy link
Contributor

AmmarCodes commented Oct 12, 2024 via email

@thewinger
Copy link
Contributor Author

thewinger commented Oct 12, 2024 via email

# - https://nikitabobko.github.io/AeroSpace/commands#list-windows
# - https://www.alfredapp.com/help/workflows/inputs/script-filter/json/

result=$(aerospace list-windows --workspace visible --format "%{app-name},%{window-title},%{window-id},%{app-pid},%{workspace},%{app-bundle-id}")

Choose a reason for hiding this comment

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

I don't have control over this PR, and the final say is after people who support the raycast plugin.

But as a maintainer of AeroSpace, the hacking around unstructured format makes me sad. Sure, you can use it for your own needs, but when you publish it for other people, I think it's better to clean it up. I'd wait for nikitabobko/AeroSpace#577 before merging this PR

I want to take this opportunity to remind that the plugin currently parses aerospace.toml config itself (which is fragile), while the proper API was provided nikitabobko/AeroSpace#215 (comment) (and still waits to be used)

This PR increases the technical debt of the raycast plugin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

List-windows.sh shouldn’t have been added. The command uses array-windows.sh but your comment still applies.
I don’t mind waiting if it will improve the code and technical debt. I started the PR before seeing the —json improvement.

@limonkufu
Copy link
Contributor

limonkufu commented Oct 12, 2024 via email

@thewinger
Copy link
Contributor Author

Closed in favor of updated version with Aerospace 0.15.0.

@thewinger thewinger closed this Oct 13, 2024
@joshgoebel
Copy link

So is this switchApps functionality coming soon then? I was just working on adding it myself - would be great if it's already in the works!

@thewinger
Copy link
Contributor Author

thewinger commented Oct 14, 2024

So is this switchApps functionality coming soon then? I was just working on adding it myself - would be great if it's already in the works!

Yes, there is already a new PR with this functionality and aerospace latest —json improvement. Feel free to look at it and add any improvements (I’m not a developer so any fix, improvement or suggestion is welcome)
#14908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension: aerospace Issues related to the aerospace extension extension fix / improvement Label for PRs with extension's fix improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants