Skip to content
This repository has been archived by the owner on Jun 17, 2023. It is now read-only.

Address #38, #39 #45

Merged
merged 3 commits into from
Feb 8, 2017
Merged

Address #38, #39 #45

merged 3 commits into from
Feb 8, 2017

Conversation

xiongtx
Copy link
Contributor

@xiongtx xiongtx commented Jan 28, 2017

Clean up Ediff buffers and restore window configuration (back to ztree)
after quitting Ediff session.

Note: I have signed the FSF's contributor agreement.

Clean up Ediff buffers and restore window configuration (back to `ztree`)
after quitting Ediff session.
ztree-diff.el Outdated
@@ -95,6 +95,8 @@ By default paths starting with dot (like .git) are ignored")
(defvar-local ztree-diff-wait-message nil
"Message showing while constructing the diff tree.")

(defvar ztree-diff--ediff-previous-window-configuration nil
Copy link
Owner

@fourier fourier Feb 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no conventions on private variables in ztree to have double dashes, please remove one.
Also please use defvar-local since we are allowing multiple ztree buffers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

You should use double dashes for private variables/functions--it's Emacs convention, and helps keep the interface small.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to flame on this, but this I see as a more "modern" convention, which is certainly not emacs-wide, as tons of build-in packages are not using it (ediff, dired, gnus etc).
Moreover there are a lot of other "conventions", i.e. ztree/myvariable etc. Maybe I'll change it at some point but I see no reason in it right now..
And in this package I would prefer the consistency in naming anyway, evening if conventions are not that convenient.

Copy link
Owner

@fourier fourier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update according to comments

Ediff registry buffer may be used by other Ediff sessions. See
magit/magit#2975.
@fourier fourier merged commit bf34366 into fourier:master Feb 8, 2017
@fourier
Copy link
Owner

fourier commented Feb 8, 2017

Thanks, merged!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants