asok / projectile-rails

Emacs Rails mode based on projectile
258 stars 59 forks source link

Does not detect my Rails app #135

Closed gdonald closed 4 years ago

gdonald commented 5 years ago

I just inherited a Rails app that is not detected by projectile-rails. It has a Gemfile in the Rails root, config/environment.rb is there, etc. Everything is normal as far as I can tell. No load errors, nothing. Any idea what else I can check? Thanks.

Silex commented 5 years ago

Is your rails app in a subfolder compared to the git repo?

If yes, I have the same problem. I meant to look after this problem since a while but never found time :disappointed:

Link to old issue for reference: https://github.com/asok/projectile-rails/issues/68

gdonald commented 5 years ago

No, it is in the root folder right alongside my .git.

Silex commented 5 years ago

Ah, that is weird then.

As you see in https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L1623-L1629 the test is pretty straight forward. What does (projectile-rails-root) return inside your project's directory?

gdonald commented 5 years ago

Placing (projectile-rails-root) at the bottom of my .emacs file doesn't do anything as far as I can tell.

Silex commented 5 years ago

Ah no, sorry I meant evaling it to check it correctly finds the rails root. Maybe this is a bit too advanced.

Are you familiar with reproducing with emacs -Q ?

gdonald commented 5 years ago

When I do M-x eval-region on it I don't get any sort of output. I've never used emacs -Q, sorry.

Silex commented 5 years ago

Ok I just tested and things seems to work fine with a default install.

What happens if you open a file from the project, enable M-x projectile-rails-mode manually, then try to M-x projectile-rails-find-model?

gdonald commented 5 years ago

I opened a model file using emacs app/models/user.rb. Then I did M-x projectile-rails-mode. At this point I begin to see 'Rails' up top. Next I did M-x projectile-rails-find-model and got the error Wrong type argument: arrayp, nil.

Silex commented 5 years ago

Can you make a zip of your project, or something similar that triggers the problem? That way we can test.

gdonald commented 5 years ago

I cannot send you this code, I do not own it.

But I made a directory, copied the Gemfile over into it and opened it, your code did not engage. I copied the app directory over and opened a model file, again your code did not engage. Same with the config directory.

Not sure how I'm supposed to know what files cause it to not work when it doesn't work with a minimal set of the most obvious files.

Silex commented 5 years ago

Can confirm it work with a rails new foobar app?

Can you also tell us your projectile/projectile-rails and rails version?

gdonald commented 5 years ago

It works fine with a rails new foobar app. It also works fine with lots of other Rails apps I have.

Rails 5.2.3. projectile-20190904.1025 projectile-rails-20190913.1003

Silex commented 5 years ago

It works fine with a rails new foobar app. It also works fine with lots of other Rails apps I have.

Okay that is interesting. You need to find out what is the difference between that app and the other apps is, or produce a testcase. You can do that by deleting almost all the controllers/models/views and hopefully the app will still bug. Then once you stripped almost everything, you can anonymize the reminder and make a zip of that for us to debug.

Before this you can try to paste a backtrace by doing M-x toggle-debug-on-error, triggering the error then copy-paste the trace here, maybe just by reading it I can figure what's wrong.

Silex commented 5 years ago

I think the difference is only in the form of the file structure.

If you look at https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L244-L252, you see we only use Gemfile and config/routes.rb to detect a rails app.

Then if you look at https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L1333-L1336 and https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L455-L461, you can see we just use patterns or jump straight to files.

gdonald commented 5 years ago

I got a backtrace when using M-x toggle-debug-on-error before projectile-rails-find-model:

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  file-truename(nil)
  projectile-rails-root-relative-to-project-root()
  #f(compiled-function (it) #<bytecode 0x40e6a7fd>)("a_issuance.rb")
  mapcar(#f(compiled-function (it) #<bytecode 0x40e6a7fd>) ("a_issuance.rb"

  [more files here]

  ...))
  projectile-rails-dir-files("/home/myuser/myapp/app/models/")
  projectile-rails-choices((("app/models/" "\\(.+\\)\\.rb$")))
  projectile-rails-find-model()
  funcall-interactively(projectile-rails-find-model)
  call-interactively(projectile-rails-find-model record nil)
  command-execute(projectile-rails-find-model record)
  execute-extended-command(nil "projectile-rails-find-model" "projectile-find-mo")
  funcall-interactively(execute-extended-command nil "projectile-rails-find-model" "projectile-find-mo")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

I see no strange characters in the filename or in the a_issuance.rb file itself. I emptied the contents of the file and still had the issue. I removed the file and still had the issue, it simply listed the next file in the directory instead.

Silex commented 5 years ago

Okay, based on your trace I think the error is at https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L793

Which means (projectile-rails-root) returns nil.

Can you open a_issuance.rb and do M-x eval-expression followed by (projectile-locate-dominating-file default-directory projectile-rails-root-file) and tell me the result?

If it returns nil, it means it cannot find your Gemfile. If it returns something, it probably means it cannot find config/routes.rb. Please check.

gdonald commented 5 years ago

This Rails app doesn't have a config/routes.rb file. It has a config/routes directory which contains many individual routes files. It uses the method described here: https://blog.arkency.com/2015/02/how-to-split-routes-dot-rb-into-smaller-parts/.

asok commented 5 years ago

@gdonald actually this is controlled by a variable projectile-rails-verify-root-file. Try putting this into .dir-locals.el in the root of your project:

;;; Directory Local Variables
;;; For more information see (info "(emacs) Directory Variables")

((nil . ((projectile-rails-verify-root-file . "config/routes/"))))
gdonald commented 5 years ago

I'm not allowed to add editor-specific files to this app.

Any other way? Something I could add to my .emacs file?

Silex commented 5 years ago

You can modify projectile-rails-verify-root-file just for this project manually, e.g M-x eval-expression (setq projectile-rails-verify-root-file "config/routes") but don't forget to set it back to the original value on other projects.

Or you can decide on another common file, like config/environment.rb or whatever railsish.

@asok: maybe we could change the default to config/environment.rb, config/boot.rb or config/application.rb? https://guides.rubyonrails.org/initialization.html

Also side note, maybe it makes more sense to have one single variable projectile-rails-root-files with values like `("Gemfile" "config/environment.rb") and whatever is there is checked, so someone can simply remove the 2nd file or add a 3rd one for more lose/strict checks.

gdonald commented 5 years ago

If it were me I'd just grep the Gemfile for /rails/.

Silex commented 5 years ago

If it were me I'd just grep the Gemfile for /rails/.

That's also a good idea.

asok commented 5 years ago

If it were me I'd just grep the Gemfile for /rails/.

This check will be run on every buffer that gets created. For that I wanted it to be as fast as possible. Checking for a presence of a file should be faster than checking for the presence of a file plus reading it. Though I'm not sure how to measure it accurately. If it turns out to be only 20% slower than I'm fine with that.

@asok: maybe we could change the default to config/environment.rb, config/boot.rb or config/application.rb? https://guides.rubyonrails.org/initialization.html

Yeah we could. The question is what would be the best? I don't know vim script but it looks like rails.vim is checking for presence of "config/environment.rb" and "app/" https://github.com/tpope/vim-rails/blob/080fc93b55102efa4bf5e46f3f247e60786641c6/plugin/rails.vim#L24

@gdonald until we make any change I think that you're best move is to add this to your init file (setq projectile-rails-verify-root-file "config/environment.rb"). That should work with your current project and any other rails project.

gdonald commented 5 years ago

Thanks, will do.

Silex commented 5 years ago

@asok: yeah in rails.vim they do that. Is it possible to have a rails app without a Gemfile? Maybe in the earlier days...

asok commented 5 years ago

I just remembered that we don't look at config/environment.rb because we also want the package to work with the rails engines projects. And them don't have this file.

Silex commented 5 years ago

I just remembered that we don't look at config/environment.rb because we also want the package to work with the rails engines projects. And them don't have this file.

Based on https://edgeguides.rubyonrails.org/engines.html, looks like what makes the most sense is:

I think checking for Gemfile existence and then grepping rails inside it (only if the file is found) would also be a decent solution, you only get the performance hit for buffers where a Gemfile is found (and that could be cached per directory in the future).

Only checking Gemfile also has the merit that for engines that don't have app, config/routes.rb or config/environment.rb it works all the time... but I'm also a bit reticent to grep inside it for performance/overkill reasons.

I think it comes down to taste, @asok I'll let you decide.

asok commented 5 years ago

What about some kind of caching? The cache would hold the list of paths to projects which are known rails projects. After opening a buffer:

Also maybe it makes more sense to grep Gemfile.lock, this has a standard format. Gemfile might have something funky like gem ENV["rails_gem_name"]. Though at the same time Gemfile.lock is not present until first bundle install.

Silex commented 5 years ago

What about some kind of caching? The cache would hold the list of paths to projects which are known rails projects. After opening a buffer:

Yeah I said we could add it in the future... but given https://github.com/asok/projectile-rails/blob/master/projectile-rails.el#L782-L783 I don't think we need to do that yet. I mean, it'd be already cached well enough by this mechanism, we can optimise it if it becomes an issue.

The crux of the issue is really how we detect the rails app :smile:

Actually, it could maybe be "trial and error", something like:

But looks like we are overengineering. We just need to fix the issue's author case and it'd be good for 99.9% of the cases.

Silex commented 4 years ago

@gdonald: #136 should have fixed your problem. Reopen if it did not.