plexus / chemacs

Emacs profile switcher
653 stars 44 forks source link

Enable the loading a profile via CLI #45

Closed ghost closed 3 years ago

ghost commented 3 years ago

Currently a profile must first be specified in "~/.emacs-profiles.el" in order to be used. In most day-to-day usage this is adequate, but on occasion it is nice to have a 'throw-away' profile. The rationale for such a profile is trying someone else's config out for a day / hour or some such, but it's not something you're committed to.

This change enables the following to work:

emacs --with-profile '((user-emacs-directory . "/tmp/throw-away-config"))'
plexus commented 3 years ago

Hey @tylerware, thank you for contributing to Chemacs.

I think this would be a fine feature to add, but I do have some concerns about this PR.

For starters the current version of Chemacs is chemacs2, I'm ok with backporting features back to the legacy chemacs, and doing bugfixes, but I don't want to add features to legacy chemacs that are not yet part of chemacs 2.

Regarding the implementation, it seems you are hacking this into a low level function (chemacs-get-emacs-profile), which doesn't seem right to me. I'd rather see us split up chemacs-load-profile into two functions, a single-arity function which takes the profile name and sets chemacs-current-emacs-profile, and a zero-arity functions which does the work of setting user-emacs-directory and friends (basically everything starting from the let*).

Then we can do the check to see if --with-profile is a name or a list in chemacs-check-command-line-args, where it belongs, and either call the function to set the profile based on the name, or just set it directly, before triggering the processing.

I would also really appreciate if you could mention this feature in the README, and the change in the CHANGELOG.

Thanks a lot!

ghost commented 3 years ago

@plexus I created a PR to chemacs2. I didn't realize there was a new version!

I think it makes sense to just include it in v2 from a versioning perspective. I definitely implemented this in a hacky way here (it worked for me) -- I wanted to see if there was openness to the feature before investing more than the minimal amount of time. I did my best to implement the feature in less of a hacky way per your guidance. LMK on the other PR if everything looks good or if I'm still missing something.

Going to close this.