-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Automatically detect changes in .spacemacs.template #2063
Conversation
This should be covered with tests probably... @syl20bnr has final say but I think tests are needed... |
@justbur I like the idea and what you propose in this PR but I find it a bit too complicated for what it achieves. I prefer the approach with a version number since it does not involve dealing with a cached file, (we know that caching is always tricky since it deals with state duplication). The version has another advantage, the version can be documentation in the change log with the corresponding changes, making it easy to follow what's new and what need to be updated. I already started to document in the change log the new dotfile changes, the dotfile version is the missing piece to be able to track precisely what need to be updated (especially for users on the develop branch). I then encourage you to update this PR by removing the cache and sha1 and replace them with a dotfile version number. You can then diff the version number in the template and in the home directory to display the message in the startup buffer. |
@syl20bnr on second thought that makes sense to me and making the version explicit may help with support even though I thought the automatic version was cool.... My next question is then how do you want to version the template? Not every Spacemacs version will correspond to a new template and it would be nice to notify only when the template does change. |
Like any emacs package but in a variable instead of comment. When a Le dimanche 21 juin 2015, Justin Burkett notifications@github.com a
-syl20bnr- |
Ok, updated. Wasn't sure if you wanted version numbers like MELPA or x.y. I thought the second was more useful in case you wanted to indicated breaking vs non-breaking changes in the template file. Also, under this scheme you will need to update the template version in two places, init.el and .spacemacs.template. This is to avoid having to look in the .spacemacs.template file on startup. |
Why are there no tests?! You are changing the core, there should be tests to cover this! |
@justbur We can fetch the version number from the template file without evaluating it so we don't have to put the version in Tests would be nice but I don't enforce them. |
@syl20bnr How's this? |
@syl20bnr Any chance you're going to accept this one? |
@justbur I like the idea and we have still no dotspacemacs version number, maybe we will never have one so this PR is even more interesting now. No doubt I will merge it. |
@syl20bnr I restructured the code a bit and made the note disappear if you click on the button. |
@justbur I'm sorry but.... can you put back the automatic detection of changes ? :-)))) (you can let the defvar for the version). While having a version number is useful, it must be used very carefully and only when the user must update her dotfile (for instance for the name change of dotspacemacs/config which will be mandatory in 0.105). But 80% of the time we will not touch the version number so.... we need your old detection algorithm... once again I'm sorry for this change of direction. |
@syl20bnr No problem. I just put the changes back |
@syl20bnr I actually think both tests make sense, so I changed this one more time. This is the logic First test for a change in the version. If this test is positive, show a note with a link to the diff. This first result will persist through restarts until the version is updated. This test can be disabled by not setting the version in your dotfile. Second test for a change between the last cached template and the current template. Again, show a note for a positive test. This is not a persistent note though and will disappear on restart (even if the user does nothing). The advantage to this approach is the persistent note if the version changes so that you can use the version to signal major changes. Minor changes are picked up by the change in the cached file (the code for this is a little simpler now too) |
First tests for a change in the version. If this test is positive, show a note with a link to the diff. This first result will persist through restarts until the version is updated. Second tests for a change between the last cached template and the current template. Again, show a note for a positive test. This is not a persistent note though and will disappear on restart (even if the user does nothing).
I thought about this a lot. Something was wrong about it but I was not able to find it but now I think I got it. First, I don't want to confuse people with another version number. Second I realized that some people have their dotfile heavily altered so the current diff between user dotfile and the template is not satisfying. Here is what I propose which I think is both simpler and more useful:
|
We can do this via git commands too by just keeping a variable that point on the last commit hash. It would be more robust. |
That's fine with me. Another completely different approach would be to add more detailed tests on the dotfile and signal to the user to run the tests on their own dotfile. For example, checking for |
This stores a hash of the current .spacemacs.template in the cache folder. When spacemacs loads, it uses this to check for changes in the template file. If a change is detected report it in the spacemacs buffer with a link to do a diff of your .spacemacs and the new one.
I have no opinion on how this information should appear in the buffer. This is how I added it, but please change it to your liking.