SystemCrafters / crafted-emacs

A sensible base Emacs configuration.
MIT License
739 stars 116 forks source link

No HOME environment variable on Windows #19

Closed sdaaish closed 2 years ago

sdaaish commented 2 years ago

The code that sets the rational-config-path in early-init.elassumes that there is a HOME-variable. But on Windows this is not true, there is no variable HOME by default.

Emacs looks for it's config in APPDATA and the user home is in USERPROFILE. Now one can set the environment variable HOME to USERPROFILE, which is also recommended.

So on Windows the early-init should look for a HOME variable and use that. If it doesn't exist it should use APPDATA.

I'm not sure why rational-emacs has to be installed in specific locations and not just use the user-emacs-directory. But that may be by design.

jeffbowman commented 2 years ago

Great information, thanks! Would you make the patch for the fix? I can do it if that is more convenient.

The separate directories are to make things easier for git locations. The eventual user could clone this repo as their emacs config and then in a separate folder clone their personal customization. There may be other ways to handle the same thing, git submodules comes to mind, but that maybe also be suboptimal in other ways. I'll defer to @daviwil on any further explanation/reasoning.

sdaaish commented 2 years ago

It would be nice if you do it, I realize I don't know exactly to handle unset variables at the moment.

jeffbowman commented 2 years ago

It would be nice if you do it, I realize I don't know exactly to handle unset variables at the moment.

@sdaaish:

PR submitted. In the way of an explanation, emacs-lisp (like other lisps) considers nil to be false. Thus (getenv "HOME") will be considered false if it returns nil. So (if (getenv "APPDATA") ...) will return t if there is a value there or nil if not. If it is present, we use it, if it isn't we default to the "HOME" environment variable. Hopefully this covers all relevant operating systems. I also check to see if the system-type is "windows-nt" as the "APPDATA" environment variable only exists on Windows (not Mac or Linux each of which uses "HOME" - which I'm sure you already knew, just trying to be obvious for posterity sake :-) )

sdaaish commented 2 years ago

@jeffbowman I tried this but now it will only look inAPPDATA and not in HOME as I have set in Windows. I realize that I may have a more complex setup so this might be an edge case.

To me Emacs should first look for config in HOME and if that is set it should continue setting the variable. Only if that doesn't exist it should look for config in APPDATA .

But it's to late for me right now to make any conclusions but I will give it a try tomorrow.

jeffbowman commented 2 years ago

@sdaaish Ah... I missed that nuance... I have updated the logic to check for HOME first, then APPDATA if on a MS Windows machine, and in the worst case just go up a directory from the init.el file.

I'm interested in your feedback. Thanks for testing!

sdaaish commented 2 years ago

@jeffbowman So it appears that I lied to you. There is in fact a HOME variable by default in Emacs on Windows. I tried rational-emacs on a new fresh installation (Windows Sandbox) and I found out that Emacs has its own HOME variable. That points to C:\Users\<UserName>\AppData\Roaming.

But you can change this by adding the environment value HOME and Emacs will respect that.

When I tried rational-emacs on the fresh Windows it worked as expected, and that surprised me. So I checked the manual. And it states this quite clearly. https://www.gnu.org/software/emacs/manual/html_node/emacs/Windows-HOME.html

So sorry for the confusion, I messed this up. I personally creates an environment variable HOME on my systems so I have forgotten how it worked originally.

So on Windows, by cloning the repository in powershell:

git clone https://github.com/SystemCrafters/rational-emacs $env:AppData/.emacs.d
runemacs.exe

works as intended with rational-emacs. So there is no need for an exception i early-init.el for Windows. At least regarding to the HOME variable.

Sorry for the confusion.

jeffbowman commented 2 years ago

@sdaaish Thanks for the update. I'll revoke the PR for this then as what we have seems to work.

@daviwil you can close this issue.

jeffbowman commented 2 years ago

With the merge of #38 can this be closed?

@sdaaish @daviwil

daviwil commented 2 years ago

Yep, thanks @jeffbowman!