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

feat: find[One]Executable in user-supplied paths #3260

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

halostatue
Copy link
Collaborator

Implement findExecutable with strong caching. The main change from
#3162 is that it now also accepts either a file or file-list for
searching, so that one of several different options can be searched
simultaneously for roughly equivalent implementations. See #3256 for the
inspiration for this particular change.

We can switch this to two template functions, but I think that the
core implementation would best remain the way that it is.

Resolves: #3141
Closes: #3162
Co-authored-by: Arran Ubels [email protected]

@halostatue halostatue force-pushed the find-executable-template-func branch 2 times, most recently from c7072b6 to 186d318 Compare September 21, 2023 14:40
Copy link
Owner

@twpayne twpayne left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

Feel free to ignore the two minor nits if you want to merge this now.

internal/chezmoi/findexecutable_darwin_test.go Outdated Show resolved Hide resolved
internal/chezmoi/findexecutable_darwin_test.go Outdated Show resolved Hide resolved
@halostatue
Copy link
Collaborator Author

One question before I merge this, @twpayne. Do you prefer findExecutable *file | file-list* *path-list* (one template function) or findExecutable *file* *path-list* and findOneExecutable *file-list* *path-list* (multiple template functions). This will affect the basic implementation I have for resolving #3256.

I think that the underlying implementation (needle, haystack any -> needle any, haystack []string) is the right one, but I can be explicit about needle []string, haystack []string in the underlying implementation and do the appropriate conversion in the template function(s).

@twpayne
Copy link
Owner

twpayne commented Sep 21, 2023

I prefer multiple template functions. This gives stronger type checking, which means bugs are found earlier.

@halostatue halostatue force-pushed the find-executable-template-func branch from 6d7a64e to 9ad2ddb Compare September 22, 2023 03:54
Implemented `findExecutable` with strong caching, extended from twpayne#3162.
Also implemented `findOneExecutable` to search for any one executable
from a list.

Resolves: twpayne#3141
Closes: twpayne#3162
Co-authored-by: Arran Ubels <[email protected]>
@halostatue halostatue force-pushed the find-executable-template-func branch from 9ad2ddb to 8cb3715 Compare September 22, 2023 05:06
@halostatue halostatue changed the title feat: findExecutable in user-supplied paths feat: find[One]Executable in user-supplied paths Sep 22, 2023
@halostatue halostatue merged commit 4a52e5c into twpayne:master Sep 22, 2023
@halostatue halostatue deleted the find-executable-template-func branch September 22, 2023 12:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lookPath should accept an optional path override
3 participants