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 parse custom label template error #23716

Closed
wants to merge 1 commit into from

Conversation

sillyguodong
Copy link
Contributor

close: #23715

image

@sillyguodong sillyguodong added type/bug outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 26, 2023
Comment on lines +74 to +77
ext := strings.ToLower(filepath.Ext(f))
if ext == ".yaml" || ext == ".yml" {
f = f[:len(f)-len(ext)]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is right?

You removed the "ext", but you still append the f to files. Could each item in files be read correctly later?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 26, 2023
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 26, 2023

I think eeyrjmr's #23717 looks better (I haven't looked into it carefully, not sure about the details)

@sillyguodong sillyguodong reopened this Mar 27, 2023
lunny pushed a commit that referenced this pull request Apr 10, 2023
Fix #23715

Other related PRs:

* #23717
* #23716
* #23719

This PR is different from others, it tries to resolve the problem fundamentally (and brings more benefits)

Although it looks like some more lines are added, actually many new lines are for tests.

----

Before, the code was just "guessing" the file type and try to parse them.

<details>

![image](https://user-images.githubusercontent.com/2114189/228002245-57d58e27-1078-4da9-bf42-5bc0b264c6ce.png)

</details>

This PR:

* Always remember the original option file names, and always use correct parser for them.

* Another benefit is that we can sort the Label Templates now (before there was a map, its key order is undefined)

![image](https://user-images.githubusercontent.com/2114189/228002432-931b9f18-3908-484b-a36b-04760c9ad132.png)
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Apr 12, 2023
…23749)

Fix go-gitea#23715

Other related PRs:

* go-gitea#23717
* go-gitea#23716
* go-gitea#23719

This PR is different from others, it tries to resolve the problem fundamentally (and brings more benefits)

Although it looks like some more lines are added, actually many new lines are for tests.

----

Before, the code was just "guessing" the file type and try to parse them.

<details>

![image](https://user-images.githubusercontent.com/2114189/228002245-57d58e27-1078-4da9-bf42-5bc0b264c6ce.png)

</details>

This PR:

* Always remember the original option file names, and always use correct parser for them.

* Another benefit is that we can sort the Label Templates now (before there was a map, its key order is undefined)

![image](https://user-images.githubusercontent.com/2114189/228002432-931b9f18-3908-484b-a36b-04760c9ad132.png)
# Conflicts:
#	modules/label/parser.go
#	routers/web/org/setting.go
#	routers/web/repo/issue_label.go
#	templates/repo/create.tmpl
#	templates/repo/issue/labels/label_load_template.tmpl
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
@sillyguodong sillyguodong deleted the bugfix/issue_23715 branch February 29, 2024 03:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

custom label option appears twice
3 participants