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

Sort root Meson files by number of path components #199

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

tristan957
Copy link
Contributor

Previous implementation was way too naive.

Previous implementation was way too naive.
@xclaesse
Copy link
Member

I think we should instead merge #195. It also fix that function, and the advantage is it does not need to glob the whole directory, in the most common case you just need to check existence in the root directory.

@wolfpld
Copy link
Contributor

wolfpld commented Nov 23, 2023

Note that I have also privately rebased #188 on top of #195, assuming that this is the way forward to continue the work. Can we agree on how to move this forward?

@tristan957
Copy link
Contributor Author

Let's move forward with #195, but I don't think it is the future of the extension. I think we really just need proper multi-root workspace support.

@tristan957
Copy link
Contributor Author

I am going to submit a bug fix release, and I would like to just merge this since it is a bug fix outside of #195. Is that fine with everybody?

@tristan957
Copy link
Contributor Author

Alternative @xclaesse could split his fix out into a separate PR that I can merge.

@wolfpld
Copy link
Contributor

wolfpld commented Nov 23, 2023

Is that fine with everybody?

It's basically what I'm also doing, so yes.

src/utils.ts Show resolved Hide resolved
@tristan957
Copy link
Contributor Author

Is that fine with everybody?

It's basically what I'm also doing, so yes.

@wolfpld definitely didn't want to steal your thunder, just didn't think it was the right place for the fix. Opened this PR is a haste to get a bug fix out, but obviously haven't merged it.

@tristan957 tristan957 merged commit 0355d9f into main Nov 28, 2023
9 checks passed
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.

3 participants