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.

Sign up for GitHub

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

Jump to bottom

Explicitly support :win-exts on which function #65

Closed
lread opened this issue May 25, 2022 · 1 comment · Fixed by #70
Closed

Explicitly support :win-exts on which function #65

lread opened this issue May 25, 2022 · 1 comment · Fixed by #70

Comments

@lread
Copy link
Contributor

lread commented May 25, 2022

Currently

@borkdude has a comment on :win-exts being unsupported for the which function:

fs/src/babashka/fs.cljc

Lines 780 to 787 in 5a63586

(defn which
"Locates a program in (exec-paths) similar to the which Unix command.
On Windows it tries to resolve in the order of: .com, .exe, .bat,
.cmd."
([program] (which program nil))
([program opts]
;; :win-exts is unsupported, if you read and use
;; this, let me know, it may break.

But...

I actually made use of :win-exts on Etaoin to help me locate .ps1 files.

So...

As requested in the comment, I reached out to borkdude on Slack.
To which he responded:

I'm fine with removing the comment and support :win-exts

Next Action

I'll follow up with a PR.
Only comment and docstring should change.
@borkdude
Copy link
Contributor

👍

lread added a commit to lread/fs that referenced this issue Aug 11, 2022
@lread
Expose :win-exts option for which function
6f7cc72 
Update docstrings for `which` and `which-all`.

Notice `which` tests are not passing on Windows
- Update test launch instruction in README to invoke existing bb `test`
task. This setup updates PATH for `which` tests.
- Address setup issue: environment var name must be `Path` for Windows.

Add Windows only test that exercises `:win-exts`.
Update some existing `which` tests to be more explicit in their expectations.

Closes babashka#65
@lread lread mentioned this issue Aug 11, 2022
Merged
borkdude pushed a commit that referenced this issue Aug 11, 2022
@lread
Fix #65: Expose :win-exts option for which function (#70)
bc38a26
lread added a commit to lread/fs that referenced this issue Aug 11, 2022
@lread
update changelog
bf29140 
Addendum for babashka#65
@lread lread mentioned this issue Aug 11, 2022
Merged
borkdude pushed a commit that referenced this issue Aug 11, 2022
@lread
update changelog (#71)
3b8010d 
Addendum for #65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Expose :win-exts option for which function lread/fs
2 participants
@borkdude @lread