Skip to content

Drop the s.el dependency #293

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

Closed
bbatsov opened this issue Feb 11, 2016 · 7 comments
Closed

Drop the s.el dependency #293

bbatsov opened this issue Feb 11, 2016 · 7 comments

Comments

@bbatsov
Copy link
Member

bbatsov commented Feb 11, 2016

I see you're depending on Emacs 24.4. I'd suggest dropping s.el and simply using subr-x.el introduced there. The fewer deps, the better.

@expez
Copy link
Member

expez commented Feb 11, 2016

Pull request welcome :)

dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el with subr-x.el

All the `thread-last' got unwound due to the re-write as s.el's `s-join'
expected separator first then the strings. Whereas subr-x.el's
`string-join' expects strings and then the separator.
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el with subr-x.el
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el with subr-x.el
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el with subr-x.el
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As part of the removal of s.el as per
clojure-emacs#293 using
functions that come as part of Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 25, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
@dotemacs
Copy link
Contributor

Hi, started on this (just in case that somebody else is thinking of tackling it)...

My progress so far: master...dotemacs:no-more-s

Comments welcome

@expez
Copy link
Member

expez commented Apr 26, 2017

Awesome, @dotemacs 👍

dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 26, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 26, 2017
As per clojure-emacs#293
replacing s.el functions with the ones that come as default with Emacs
@benedekfazekas
Copy link
Member

looks cool!

will try to find time to review in detail but so far looks really good. also not planning to merge this before releasing 2.3.0

@benedekfazekas
Copy link
Member

what is the problem with using thread-last here? master...dotemacs:no-more-s#diff-ad8315941dfda54547f9762bd5a52d04L305

@dotemacs
Copy link
Contributor

Hey @benedekfazekas

I knew that this question would come up :)

So I put the comment about it in the commit, pasting it here so that you don't have to go looking for it

All the thread-last got unwound due to the re-write as s.el's s-join
expected separator first then the strings. Whereas subr-x.el's
string-join expects strings and then the separator.

dotemacs@da35e4b

So it was a case of should I keep thread-last just to keep it? Or should I write a function that uses string-join internally but takes arguments as s-join and reorders them for string-join?
I figured that just unwinding thread-last was the easiest option.
But if you have a prefered way of doing this, then please do share :)

Also, there are 2-3 functions more from s.el that I still need to re-write, that's why I haven't created the pull request just yet. I only put a comment here that I'm working on it, so that somebody else doesn't pick it up...

Should I create a pull request now, if we're to have more discussion about the code?

As for when you'll merge this, no rush on my side.

@benedekfazekas
Copy link
Member

ha, makes sense. sorry missed the commit comment.

PR: yes please. would be easier to comment stuff, etc.

dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 27, 2017
As per the issue
clojure-emacs#293 removed s.el
so that there is one less dependency for this mode that isn't part of
the standard Emacs release.
dotemacs added a commit to dotemacs/clj-refactor.el that referenced this issue Apr 27, 2017
As per the issue
clojure-emacs#293 removed s.el
so that there is one less dependency for this mode that isn't part of
the standard Emacs release.
benedekfazekas pushed a commit that referenced this issue May 2, 2017
* Replaced s-join with string-join

As per #293
replacing s.el with subr-x.el

All the `thread-last' got unwound due to the re-write as s.el's `s-join'
expected separator first then the strings. Whereas subr-x.el's
`string-join' expects strings and then the separator.

* Replaced s-trim with string-trim

As per #293
replacing s.el with subr-x.el

* Replaced s-blank? with string-blank-p

As per #293
replacing s.el with subr-x.el

* Replaced s-concat with concat

As per #293
replacing s.el with subr-x.el

* Replace s-starts-with-p with string-prefix-p

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replace s-(suf|pre)fix-p with string-(suf|pre)fix-p

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replaces s-equals-p with string-equal

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replaced s-ends-with-p with string-suffix-p

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replaced s-replace

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replaced s-matches-p

As per #293
replacing s.el functions with the ones that come as default with Emacs

* cljr--project-dir returns blank string instead of nil

When using cljr--project-dir with the stock Emacs function
string-remove-prefix, like this:

  (string-remove-prefix (cljr--project-dir) path)

instead of s.el's s-chop-prefix:

  (s-chop-prefix (cljr--project-dir) path)

it'll bomb out because cljr--project-dir will return nil, like it does
when running the tests.

But if it returns a blank string, then there is no issue.

* Replaced s-split with split-string

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Remove s-chop-(pre|suf)fix with string-remove-(pre|suf)fix

* Removed s-(pre|ap)pend

* Switch to downcase from s-downcase

As part of this issue:

#293

* Remove s-present?

As part of the removal of s.el as per
#293 using
functions that come as part of Emacs

* Replaced s-contains-p

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replace s-match

As per #293
replacing s.el functions with the ones that come as default with Emacs

* Replace s-dashed-words

As part of the drive to replace the dependency on the functions from
s.el replacing s-dashed-words with function that relies only on
functions that come as part of Emacs release.

* Replaced s-with

thread-last seems to be doing the same thing and is part of Emacs'
subr-x.el whereas s-with is part of s.el

* Replace s-slice-at

As part of the attempt to remove the dependency on s.el, replaced
s-slice-at with a simpler/cruder function.

* Less parens

Removed the unnecessary parenthesis within thread-last macros

* Removed s.el as a dependency

As per the issue
#293 removed s.el
so that there is one less dependency for this mode that isn't part of
the standard Emacs release.

* Remove s-lines

Replacing it with split-string, but with the parameter taken from
s-lines

* Created cljr--string-present-p

As part of the extraction of the dependencies on s.el, it was replaced
with Emacs native functions, but then it was deemed that their usage was
not straightforward enough, hence cljr--string-present-p

* Cleaning up the intent

Onliner instead of if statement

* Fix for the omission with split-string

It should have been a thread first since the switch from s.el's s-split
takes regexp as the first argument and the string as the last. Whereas
split-string takes the arguments the other way around.

* Made cljr--whitespacep null safe

After removing s-blank? from s.el, just using string-blank-p wasn't null
safe.

Added the null check on Benedek Fazekas' prompting.

* Let diet

No need for the let clause, removing it

* Reused cljr--string-present-p

Using it in a few more places

* Avoiding repetition

Used mapcar in order to avoid the repetition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants