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

Added recipe for projectable #2959

Closed
wants to merge 1 commit into from
Closed

Conversation

domtronn
Copy link
Contributor

Github: https://github.com/domtronn/projectable.el

This is my own lightweight project framework package.

It allows you to define projects in JSON and eases navigation to and from files.

Thanks!

@purcell
Copy link
Member

purcell commented Aug 23, 2015

Sorry, I totally missed this PR. Quick feedback now:

  • Please use :defaults in the :files list in place of "projectable.el"
  • Don't bind C-c p -- that's reserved for users. By all means suggest this binding to users in the documentation.
  • Instead of projectable-ctags-supported-languages, consider looking through auto-mode-alist
  • Instead of projectable-filter-regexps, consider using completion-ignored-extensions and/or grep-find-ignored-directories/grep-find-ignored-files
  • projectable-is-file invokes the shell? Why not file-directory-p?
  • projectable-load-from-json uses cat to load json? This can be done easily (and more portably) in elisp.
  • Everywhere you construct a command string, use shell-quote-argument to make sure paths containing spaces don't break the command or the user's system
  • To use setq-default, you'll need to declare a package dependency on Emacs 24.3 or greater
  • You should add ;;;###autoload cookie comments for the mode definition forms
  • Mode lighters for minor modes should start with a space, so try " P>%s"
  • Don't require a bunch of stuff with noerror: just because the user has these packages installed doesn't mean he/she wants to use them. Instead, use eval-after-load.
  • Don't write out messages at load time (projectable-message)
  • The docstring for projectable-project-directory is misleading: the default is not what the docstring says it is
  • find-file-upwards should be prefixed with projectable, and could be much more easily implemented using locate-dominating-file

Overall I'd have to say this package is still a little muddled: I don't think it will be clear to many potential users what it would do for them, and the name makes it sound like projectile, with which I can't see much similarity. My feeling is that much of this code is very personal to your own workflow, and might therefore not be a good candidate for packaging.

Hope that helps,

-Steve

@purcell purcell added recipes awaiting-upstream Awaiting action from an upstream maintainer labels Aug 23, 2015
@milkypostman
Copy link
Member

ping

@domtronn
Copy link
Contributor Author

Hi, thanks for all the feedback! I'm still very much working on this project and all that information has been really useful for improving it in ways I was un aware of. I've raised issues on the project for each of the points as a way to monitor its progress.

Yeah, w.r.t. the name, I'm not overly keen on it, but that's subject to change I guess.

But I feel like it serves a purpose that differentiates it from projectile, but don't think I explained it very well. I feel like the advantage of this package is how it's declarative. Where projectile works with source control mechanisms to define/create projects, this package lets you declare (in json) the folders that you care about. This is very similar to the way modern text editors define projects, either opening a folder as a project or defining the components of your projects in json.

I did write this very much with my own workflow in mind, however there are a few people who prefer this (for whatever reason) to other project solutions so I'm keen to maintain and support them.

I'll update this issue when I feel I've addressed all of the notes in the previous comments, thanks for the support :)

@AlexChesters
Copy link

In addition to @domtronn's point, I find the ability to declare libraries particularly useful

@purcell
Copy link
Member

purcell commented Oct 15, 2015

I did write this very much with my own workflow in mind, however there are a few people who prefer this (for whatever reason) to other project solutions so I'm keen to maintain and support them.

I'll update this issue when I feel I've addressed all of the notes in the previous comments, thanks for the support :)

Yay, great. Just ping us when ready. :-)

@tarsius
Copy link
Member

tarsius commented Aug 14, 2016

@domtronn has renamed projectable to jpop. The current version looks okay to me, so I have opened a new pr for that #4139.

@tarsius tarsius closed this Aug 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-upstream Awaiting action from an upstream maintainer recipes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants