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

Detect the root of Subversion checkouts. #291

Merged
merged 1 commit into from Mar 28, 2014
Merged

Conversation

Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Mar 23, 2014

Related to #285, it would be nice if projectile could automatically detect the root of subversion repositories. This code provides that.

I've added a dependency on the excellent f.el, since I think it makes the code much more readable. Happy to refactor if you don't want the dependency.

Let me know if you want me to make any improvements :)

@bbatsov
Copy link
Owner

bbatsov commented Mar 23, 2014

The code looks good, but I don't think we need f.el. Most of the functions you've used in your are trivial wrappers around the "native" API and the predicate methods in f.el use the pretty unemacsy suffix ? (blame it on @magnars and his love for Clojure (which I share) I guess :-) ). Personally I prefer it over the canonical -p, but I value consistency above all else. s.el at least has aliased versions of predicate methods. No idea why @rejeep decided to disregard the common convention (although there's f-unibyte-string-p).

@Wilfred
Copy link
Contributor Author

Wilfred commented Mar 26, 2014

Searching through the functions defined in my current Emacs instance, you're right about ?. The only packages using ? for predicates (that I have installed) are dash.el, s.el, f.el and ht.el. It makes sense to have -p aliases. I've never liked the convention of foop without the dash though.

Anyway, I've pushed a commit that removes the f.el dependency. It's not quite as robust (f.el does the right thing with path expansion and relative paths) but sufficient for projectile's needs. I think the code reads slightly more nicely with f.el, but now you can choose which you like best :).

@bbatsov
Copy link
Owner

bbatsov commented Mar 27, 2014

Thanks for the update. Just add a changelog entry and squash the two commits together.

@Wilfred
Copy link
Contributor Author

Wilfred commented Mar 27, 2014

Squashed and pushed.

bbatsov added a commit that referenced this pull request Mar 28, 2014
Detect the root of Subversion checkouts.
@bbatsov bbatsov merged commit 661c16a into bbatsov:master Mar 28, 2014
@bbatsov
Copy link
Owner

bbatsov commented Mar 28, 2014

Thanks!

@thomasf
Copy link
Contributor

thomasf commented Mar 29, 2014

maybe locate-dominating-stop-dir-regexp should be consulted too for consistency with top-down traversals.

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.

None yet

3 participants