Skip to content

Fix umbrella app detection #88

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

Merged
merged 1 commit into from
May 13, 2023
Merged

Fix umbrella app detection #88

merged 1 commit into from
May 13, 2023

Conversation

davidelias
Copy link
Contributor

This fixes umbrella path detection. Currently, maybe_umbrella_path is always the same as child_or_root_path because of how vim.fs.find works.
So this change kind of restores the logic that existed before removing lspconfig dep

Suggestion

It also removes the logic that detects if the umbrella path contains the dir apps. This was because this path could be set to another name in mix. exs with :apps_path, and maybe it is redundant checking for this when we found the mix. exs in a parent dir. But this is a suggestion, and I may be missing something.

@davidelias davidelias changed the title fix: umbrella app detection Fix umbrella app detection May 7, 2023
Copy link
Collaborator

@mhanberg mhanberg left a comment

Choose a reason for hiding this comment

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

This looks good! I'll merge if you're done making changes, just let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is awesome!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've just rebased with main, so it should be good to merge 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent

@mhanberg
Copy link
Collaborator

Can you rebase on main actually?

- adds tests for `elixir.utils.root_dir`
@mhanberg mhanberg enabled auto-merge (squash) May 13, 2023 16:57
@mhanberg mhanberg disabled auto-merge May 13, 2023 16:57
@mhanberg mhanberg enabled auto-merge (squash) May 13, 2023 16:57
@mhanberg mhanberg merged commit a17c863 into elixir-tools:main May 13, 2023
@mhanberg
Copy link
Collaborator

Thanks again! 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants