Skip to content

split match by index and match by name#23

Open
suqin-haha wants to merge 1 commit into
muir:mainfrom
suqin-haha:excludeMatchTagByIndex
Open

split match by index and match by name#23
suqin-haha wants to merge 1 commit into
muir:mainfrom
suqin-haha:excludeMatchTagByIndex

Conversation

@suqin-haha
Copy link
Copy Markdown
Contributor

@suqin-haha suqin-haha commented Oct 14, 2024

struct {
    	T6 bool   `xyz:"first,first" want:"{\"Name\":\"first\",\"First\":true}"`
        T7 bool   `xyz:"first,!first" want:"{\"Name\":\"first\",\"First\":false}"`
}

fix delete(kv, value) will make second first never match.

ref: #22 (comment)

Comment thread parsetag.go
return err
}
// 2.remove matched by index
newElements := []string{}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
newElements := []string{}
newElements := make([]string, 0, len(elements))

Comment thread parsetag.go
Comment on lines +169 to +175
newElements := []string{}
for i, e := range elements {
if elementsConsumed[i] {
continue
}
newElements = append(newElements, e)
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
newElements := []string{}
for i, e := range elements {
if elementsConsumed[i] {
continue
}
newElements = append(newElements, e)
}
newElements := []string{}
for i, e := range elements {
if elementsConsumed[i] {
continue
}
newElements = append(newElements, e)
}
elements = newElements

just use this to filter elements, you don't need a new variable exposed to the rest of the function.

Comment thread parsetag.go
value = elements[i]
delete(kv, value) // exclude from rest match
} else {
_, err := strconv.Atoi(parts[0])
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't like the repeat call to Atoi. I'm not sure it can be avoided though.

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