vim-erlang / vim-erlang-runtime

Erlang indentation and syntax for Vim
https://vim-erlang.github.io
101 stars 29 forks source link

Add common Erlang dirs to search path #51

Closed lukebakken closed 2 years ago

lukebakken commented 2 years ago

Related to https://github.com/elixir-editors/vim-elixir/pull/543

hcs42 commented 2 years ago

Hi,

What is the use case? For example gf for include_lib("filename.erl")?

Which build system do you use? I use rebar3 these days, and it has different directories (e.g., is has _build instead of deps, and the umbrella projects also have an apps directory).

I think that if we include this, we should have an option for it, e.g. g:erlang_extend_path. We can set it to 1 by default if we think that it's useful for most people. See the other options here: https://github.com/vim-erlang/vim-erlang-compiler/blob/b334e956026f61c0bf289ffdf37ce9b2aefe01e1/doc/vim-erlang-compiler.txt#L57

lukebakken commented 2 years ago

What is the use case? For example gf for include_lib("filename.erl")?

I frequently use the find, sf and vert sf commands to open files that I know are in either src/ or deps/.

Which build system do you use?

That's a good point. I'm a member of the RabbitMQ core engineering team. RabbitMQ uses erlang.mk. I also work with rebar based projects and it would make sense to update the search directories with rebar-specific ones. I'll do that.

I also like the idea of an option, I'll get that as well.

Thanks for the review!

lukebakken commented 2 years ago

@hcs42 I've added the rebar directories and configuration setting. It would be great to make the path addition smart enough so that for a project named foobar the path _build/default/lib/foobar isn't added to the search path. I'll see if I can figure out the vimscript for that.

I also need to add tests. Let me know how it looks so far if you have a second!

hcs42 commented 2 years ago

It looks good so far. (I haven't tested it yet though.)

A few comments:

  1. "It would be great to make the path addition smart enough so that for a project named foobar the path _build/default/lib/foobar isn't added to the search path. I'll see if I can figure out the vimscript for that." → For umbrella projects, I think this would need to be applied to all foobar applications in the apps directory. How about just modifying the order to ensure that apps directories come first:

    let &l:path = join(['apps/*/include',
                \       'apps/*/src',
                \       '_build/default/lib/*/src',
                \       '_build/default/*/include', &l:path], ',')

    This way :find and other commands would work correctly even if foobar is present in both apps and _build.

  2. I try to have maximum 80 character long lines where possible. (I haven't reformatted all existing long lines.)

  3. Feel free work in one commit and amend that when you make a change. I prefer merging most contributions as one commit in the end.

  4. Could you update the documentation too before finalizing the PR? It would be good to mention that this feature depends on setting the current directory (:help current-directory) to the project's or application's root folder. This is usually not the case in my development workflow (I run only one GVim instance, and do all of my editing inside that). However, I know that there are users who start Vim from the project directory.

lukebakken commented 2 years ago
  1. Great suggestion.
  2. Yep, I'll re-format before rebasing.
  3. Will do. I planned on squashing before the final merge.
  4. Absolutely!
lukebakken commented 2 years ago

@hcs42 - ready for a re-review, thank you.

hcs42 commented 2 years ago

@lukebakken Thank you for your contribution!