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

fix: Always prefer tauri configs further up the file tree #502

Merged
merged 8 commits into from
Jul 25, 2023

Conversation

FabianLars
Copy link
Member

@FabianLars FabianLars commented Jun 16, 2023

This fixes issues where the action used config files in deeply nested directories instead of the correct one in the upper levels.

ref: https://discord.com/channels/616186924390023171/1119317689836515388

kinda quick and dirty, have to call it a day. If it works for the user in that thread too i'll try to clean it up a bit. (or maybe not as i don't want to have a helper function just to reuse that inner for loop)

Edit: User confirmed it worked for them, therefore converting it to a normal PR, still gonna plan to try to clean it up a bit but feel free to have a look.

This fixes issues where the action used config files in deeply nested directories instead of the correct one in the upper levels.
@socket-security
Copy link

socket-security bot commented Jun 16, 2023

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
globby 13.2.2 None +2 126 kB sindresorhus

🚮 Removed packages: glob-gitignore@1.0.14

@FabianLars FabianLars marked this pull request as ready for review June 17, 2023 09:39
@FabianLars
Copy link
Member Author

okay nevermind about cleaning it up, i'd rather keep it as simple&stupid as possible for now.

Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Looking at the code, it seems that this is only traversing down and not up, right? if so I fail to see how is this different from the previous impl

@FabianLars
Copy link
Member Author

take for example this file tree, taken from https://github.com/tonymushah/special-eureka/tree/master/src-tauri:

/
  src-tauri/
    examples/
      tauri.conf.json
    src/
    tauri.conf.json # this is the file we want

The current implementation will always select src-tauri/examples/tauri.conf.json while the new one will go for src-tauri/tauri.conf.json -> tested by me and confirmed by 2 community members one of which linked above.

One variant i didn't think about would be this

/
  src/
  some-package/
    whatever/
      tauri.conf.json
  src-tauri/
    tauri.conf.json # this is the file we want

Here the new implementation will also go for the wrong file. I can't remember if setting projectPath to the src-tauri dir and not its parent works too so that would need some testing too ig

@amrbashir
Copy link
Member

I see but as you outlined this fix is not complete as it doesn't account for levels at all, I'd recommend reverting this change and use glob again but instead of taking the first result, we sort the return paths first by how far are they to the current working directory.

@FabianLars
Copy link
Member Author

Do you have any pointers on how to sort them then? Because i didn't find an easy-ish & reliable way to do that which is really why this PR exists in the first place.

@amrbashir
Copy link
Member

amrbashir commented Jul 11, 2023

if we are looking down only, then it should be easy to just split on path separator and count the components of the path, and choose the lowest number

@FabianLars
Copy link
Member Author

yeah, that sounded kinda scary in my head but if you also have that in mind we can try it out and keep an eye on it, thanks 👍

@FabianLars FabianLars changed the title fix: add custom file search for more consistent results. fix: Always prefer tauri configs further up the file tree Jul 25, 2023
@FabianLars
Copy link
Member Author

I found an old todo entry to evaluate globby since it can handle gitignore files dynamically (i mean it adds gitignore files along the way, not just the ones we give it upfront) so i switched to that. And apparently it already sorts the results correctly in contrast to really all the other glob/file search packages i tried 🥳

I'll keep an eye on it though and re-add the sort() call if necessary.

.changes/enhanve-tauridir-search-gitignore.md Outdated Show resolved Hide resolved
src/utils.ts Show resolved Hide resolved
@FabianLars FabianLars merged commit c87af54 into dev Jul 25, 2023
3 checks passed
@FabianLars FabianLars deleted the custom-search branch July 25, 2023 14:37
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.

None yet

2 participants