search

iquiw / company-ghc

Company-mode completion back-end for haskell-mode via ghc-mod
125 stars 6 forks source link

Add package dependencies #1

Closed purcell closed 10 years ago

purcell commented 10 years ago

I plan to add this package to MELPA and MELPA Stable, so these dependencies are required. Note that the emacs "24.4" dependency is due to your use of cl-case: if you can replace that with something more traditional (and remove lexical-binding: t), then the code would probably work fine with Emacs 23 and older 24.x releases, which I think would be a good idea.

bbatsov commented 10 years ago

company requires Emacs 24.1, so removing lexical binding is redundant. Isn't cl-case in cl-lib anyways? The "more traditional" option is pcase I guess.

purcell commented 10 years ago

Ah yes, cl-case does seem to get defined in cl-lib 0.5: my initial cursory check indicated it wasn't. My mistake!

pcase is a less-compatible alternative, having only been added recently.

Regarding lexical-binding, it's redundant if this code doesn't require it, regardless of the dependencies of its dependencies: any package which directly contains code that needs Emacs 24.x should declare a dependency on that Emacs version, and not rely on transitive dependencies.

I'll happily fix up this PR in one of two ways, then, and I'm happy to hear which would be preferred:

  1. Remove lexical-binding and dependency on emacs 24.4
  2. Retain lexical-binding and depend on emacs 24.
iquiw commented 10 years ago

I added lexical-binding: t just because I am familiar with languages that have lexical binding. I have not much knowledge on Emacs lisp, so I am open to the choices.

purcell commented 10 years ago

@iquiw In the vast majority of code, the resulting behaviour is identical. So usually you can omit lexical-binding and be confident that the code will work the same in Emacs 23.x. Once lexical-binding is present, it's no longer guaranteed that a piece of code will work the same in Emacs 23.x, because it may be interpreted differently.

In the case of this package and your preferences, I'd suggest going with my option 2 above, because there's no chance that this package would work in Emacs 23.x.

iquiw commented 10 years ago

@purcell Thank you for the explanation.

In the case of this package and your preferences, I'd suggest going with my option 2 above, because there's no chance that this package would work in Emacs 23.x.

It's fine for me.

purcell commented 10 years ago

Replaced by #2