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

emacs-plus@29: inject PATH variable into Info.plist file #453

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

d12frosted
Copy link
Owner

The core issue here is environment in which
macOS starts applications from Finder/Docker/Spotlight/etc. This
change mostly helps users of native-comp feature (e.g. #378), but
it also should help other users as well (see this comment)

The core issue here is environment in which
macOS starts applications from Finder/Docker/Spotlight/etc. This
change mostly helps users of `native-comp` feature (e.g. #378), but
it also should help other users as well ([see this comment][1])

[1]: #414 (comment)
@d12frosted
Copy link
Owner Author

Will merge it on Wednesday unless someone reports issues.

@d12frosted d12frosted merged commit edb5b25 into master Apr 27, 2022
@d12frosted d12frosted deleted the feature/inject-path branch April 27, 2022 03:48
@d12frosted d12frosted mentioned this pull request May 9, 2022
d12frosted added a commit that referenced this pull request May 10, 2022
It's the first step towards PATH injection to Emacs.app as it is done
in emacs-plus@29 (see #453).

It has changes from #453, #457, #460 and #462.

Next step would be to actually inject PATH as it is done in #453.
d12frosted added a commit that referenced this pull request May 10, 2022
It's the first step towards PATH injection to Emacs.app as it is done
in emacs-plus@29 (see #453).

It has changes from #453, #457, #460 and #462.

Next step would be to actually inject PATH as it is done in #453.
d12frosted added a commit that referenced this pull request May 10, 2022
It's the first step towards PATH injection to Emacs.app as it is done
in emacs-plus@29 (see #453).

It has changes from #453, #457, #460 and #462.

Next step would be to actually inject PATH as it is done in #453.
d12frosted added a commit that referenced this pull request May 10, 2022
It's the first step towards PATH injection to Emacs.app as it is done
in emacs-plus@29 (see #453).

It has changes from #453, #457, #460 and #462.

Next step would be to actually inject PATH as it is done in #453.
d12frosted added a commit that referenced this pull request May 12, 2022
d12frosted added a commit that referenced this pull request May 12, 2022
@gildo
Copy link

gildo commented May 16, 2022

hi @d12frosted, is this patch removing the support to brew service from emacs-29?

@d12frosted
Copy link
Owner Author

@gildo no, this simply injects PATH variable value to Info.plist file as the title says. You can read more about this feature in the separate section of readme file. Why do you think that it's related to brew services? And most importantly, why do you think I am removing support?

@roman-rudakov
Copy link

@d12frosted as far as I can see, the def plist section is removed, and when I build emacs-plus@29 I don't see emacs-plus@29 in the brew services list anymore.

@d12frosted
Copy link
Owner Author

@roman-rudakov you are right. I will restore this plist. Not sure what I was thinking when removing this plist. Thanks for explanation!

d12frosted added a commit that referenced this pull request May 17, 2022
@d12frosted
Copy link
Owner Author

@roman-rudakov see #467

@gildo
Copy link

gildo commented May 17, 2022

@d12frosted sorry I had not explained myself correctly. I'm not used to homebrew formulae and I thought that you accidentally removed the feature, but I wasn't sure.

Thanks for your work,
Gildo

d12frosted added a commit that referenced this pull request May 17, 2022
@d12frosted
Copy link
Owner Author

@gildo no worries :) I just didn't understand how it was related. Sorry about that!

@roman-rudakov and @gildo I merged the fix.

@roman-rudakov
Copy link

Thank you @d12frosted!

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