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

core: Add fs-basename function #196

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

danielbayley
Copy link

# ~/path/to/file.ys
filename:: fs-basename(FILE) # file.ys
basename:: fs-basename(FILE "ys") # file

@ingydotnet
Copy link
Member

Hi. Thanks for trying out YS and thanks for the PR.
Love collaboration!

I'm not entirely opposed to this, but these already work:

$ ys -pe "fs-filename('$PWD/ReadMe.md')"
"ReadMe.md"
$ ys -pe "fs-filename('$PWD/ReadMe.md').replace('.md')"
"ReadMe"

Tell me a bit why you'd prefer this if you don't mind.

@ingydotnet
Copy link
Member

@danielbayley I added a couple commits to your work here https://github.com/yaml/yamlscript/tree/fs-basename

Let me know what you think.

@danielbayley
Copy link
Author

danielbayley commented Dec 3, 2024

Tell me a bit why you'd prefer this if you don't mind.

@ingydotnet Sure, so I was going on the Ruby basename method, which has an optional second param to remove the given extension, or more usefully, has the ability to remove any extension with a basic glob:

# ~/path/to/file.ys
fs-basename(FILE ".*") # file

these already work:

$ ys -pe "fs-filename('$PWD/ReadMe.md').replace('.md')"
"ReadMe"

As does this:

fs-filename(FILE):fs/strip-ext

But it’s still 2 methods, rather than one. If I’m going to mix code directly into my data, I want it to be as terse as possible (without losing legibility).

Also on that note, how come fs/strip-ext, and fs/split-ext are not aliased to fs-stripext/fs-splitext respectively, and included with the other fs-* methods, as listed here?

I also thought about adding this second param to the existing fs-filename method, and/or just having fs-basename as a straight up alias to that…

What do you think? @ingydotnet

Further reading on this: babashka/fs#28, and babashka/fs#29.

@ingydotnet
Copy link
Member

If I’m going to mix code directly into my data, I want it to be as terse as possible (without losing legibility).

We have shared goals there!!

I'll look it all over in a bit and push some commits for you to review

@ingydotnet
Copy link
Member

We don't need to alias fs/split-ext because we can just use that directly, no?

$ ys -pe "fs-filename('$PWD/ReadMe.md'):fs/split-ext"
["ReadMe" "md"]

@ingydotnet
Copy link
Member

I'll add support for

fs-basename(FILE '*')

note that fs/strip-ext doesn't use a . in the ext.

@danielbayley
Copy link
Author

We don't need to alias fs/split-ext because we can just use that directly, no?

Well yes, but then why alias some and not others? I’m just thinking about consistency is all…

@ingydotnet
Copy link
Member

We don't need to alias fs/split-ext because we can just use that directly, no?

Well yes, but then why alias some and not others? I’m just thinking about consistency is all…

afaict, none of std/fs-foobar match fs/foobar exactly.

Some are close like fs-filename because I prefer filename to file-name.

ingydotnet added a commit that referenced this pull request Dec 14, 2024
@ingydotnet
Copy link
Member

Pushed 14a53e6 to devel branch.

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