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

don't require saveplace #65

Merged
merged 1 commit into from
Oct 13, 2013
Merged

don't require saveplace #65

merged 1 commit into from
Oct 13, 2013

Conversation

npostavs
Copy link
Contributor

@npostavs npostavs commented Oct 1, 2013

If the user hasn't setup saveplace in their config, requiring saveplace
causes them to get a useless .emacs-places in the home directory. To
avoid triggering a compilation warning, we check if save-place is
bound.

f5b36db had changed from calling toggle-save-place to setting
save-place but still checked if toggle-save-place was fbound,
830b538 then required saveplace to quiet the compiler.

If the user hasn't setup saveplace in their config, requiring saveplace
causes them to get a useless .emacs-places in the home directory. To
avoid triggering a compilation warning, we check if `save-place' is
bound.

f5b36db had changed from calling `toggle-save-place' to setting
`save-place' but still checked if `toggle-save-place' was fbound,
830b538 then required saveplace to quiet the compiler.
@PureAbstract
Copy link
Contributor

It may be worth noting that toggle-save-place 0 has been broken for quite some time (certainly as far back as I can find history for it). Specifically, if save-place is nil, toggle-save-place turns it on regardless of the argument. I submitted a patch to the Emacs maintainers a few years ago, but for whatever reason it never seemed to make it into the tree. (It made it into Aquamacs though - aquamacs-emacs/aquamacs-emacs@3be78ea)

That's not terribly important here, though, since you're setting save-place directly.

@praet
Copy link
Contributor

praet commented Oct 9, 2013

On Mon, 30 Sep 2013 18:17:23 -0700, Noam Postavsky notifications@github.com wrote:

-- Commit Summary --

  • don't require saveplace

LGTM.

Reviewed-by: Pieter Praet pieter@praet.org

Peace

Pieter

tarsius added a commit that referenced this pull request Oct 13, 2013
@tarsius tarsius merged commit 46ab348 into magit:master Oct 13, 2013
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.

4 participants