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 path.Match, missing param when registering #3201

Closed
wants to merge 1 commit into from

Conversation

verdverm
Copy link

@verdverm verdverm commented Jun 6, 2024

Feel free to make this one-line change on your own clone if it will be much faster and make v0.9


Fixes input parameter handling

https://cuelang.org/play/?id=pJ-fBYTb409#w=function&i=cue&f=eval&o=cue

import "path"

a: path.IsAbs("/foo")
a: path.IsAbs("/foo", "unix")
b: path.Split("/a/b/c")
b: path.Split("/a/b/c", "unix")
c: path.Rel("/a/b/c", "/a/b")
d: path.Match("/a/b/c", "/a")

// Expected way to use
// e1: too many arguments in call to path.Match (have 3, want 2)
// e1: path.Match("foo.cue", "*.cue", "unix")

// so remove os...?
// cannot use "*.cue" as *"unix" | "windows" | "plan9" | "aix" | "android" | ...
// e2: path.Match("foo.cue", "*.cue")

// is this a constraint only?
// error in call to path.Match: runtime error: index out of range [2] with length 2
// e3: "foo.cue" & path.Match("*.cue", "unix")

@verdverm verdverm requested a review from cueckoo as a code owner June 6, 2024 02:18
verdverm pushed a commit to hofstadter-io/hof that referenced this pull request Jun 6, 2024
…k of flows to replace make rules

      also temporarily replace cue until cue-lang/cue#3201 is fixed
@mvdan
Copy link
Member

mvdan commented Jun 6, 2024

Thanks! This would need tests, and for you to sign off the commit. As it stands, it cannot really be reviewed nor merged, so this will have to wait until v0.9.1.

Copy link
Member

@mvdan mvdan left a comment

Choose a reason for hiding this comment

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

Meant to mark as "changes requested".

@verdverm verdverm closed this Jun 6, 2024
@verdverm
Copy link
Author

verdverm commented Jun 6, 2024

Converting to an issue so the CUE team can fix this

#3203

@verdverm
Copy link
Author

verdverm commented Jun 6, 2024

I've also signed the DCO previously, not sure why GH is not detecting it

@verdverm
Copy link
Author

verdverm commented Jun 6, 2024

@mvdan
Copy link
Member

mvdan commented Jun 6, 2024

DCO, i.e. a signed-off-by trailer, has to be added to every commit to be merged. I'll continue over on Noam's patch.

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