Skip to content

Conversation

@skovhus
Copy link
Collaborator

@skovhus skovhus commented Mar 2, 2020

Context

Issue: #47

This builds on the work done a year ago (#113). I ended up re-implementing it to get back into the problem space. Thanks to @przepompownia for re-booting this (#156).

This PR

Here we add support for configuring the glob pattern used when pre-analyzing all bash files (this is done to support jump to definition). Besides making it configurable we extend it from **/*.sh to **/*@(.sh|.inc|.bash|.command).

I recommend reviewing commit by commit.

Next steps

This will introduce a few false positives as we do not parse the shebang or mime types (yet), as I tried to document in the fixture folder.

Next step might be to implement the shebang part of #113 or mime types (#47 (comment)) and consider the ideas introduced in #156

But the challenge is that not everyone use shebangs in their bash files... so I guess we shouldn’t make that a filter?

If we can get the included files out of tree sitter, then we could potentially parse those files.

@skovhus skovhus requested a review from mads-hartmann March 2, 2020 12:47
Copy link
Collaborator

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you @skovhus 🎉

@skovhus
Copy link
Collaborator Author

skovhus commented Mar 2, 2020

A few stats to guide upcoming work (shebang/mimetype handling):

Globbing files inside our node_modules (just to have a bunch of files):

  1. **/*@(.sh|.inc|.bash|.command) and **/*.sh resolves in 3 seconds (with 3 files)
  2. **/* resolves in 5 seconds with 14K files

Reading the file content (sync) of the 14K files takes around 6 seconds. This would need to be done if we analyze the shebang of all files.

@skovhus skovhus merged commit bba68bb into master Mar 2, 2020
@skovhus skovhus deleted the improve-globbing branch March 2, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants