Skip to content

Add crlf to lf conversion to swagger generation #33613

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ SWAGGER_SPEC := templates/swagger/v1_json.tmpl
SWAGGER_SPEC_S_TMPL := s|"basePath": *"/api/v1"|"basePath": "{{AppSubUrl \| JSEscape}}/api/v1"|g
SWAGGER_SPEC_S_JSON := s|"basePath": *"{{AppSubUrl \| JSEscape}}/api/v1"|"basePath": "/api/v1"|g
SWAGGER_EXCLUDE := code.gitea.io/sdk
SWAGGER_NEWLINE_COMMAND := -e '$$a\'

TEST_MYSQL_HOST ?= mysql:3306
TEST_MYSQL_DBNAME ?= testgitea
Expand Down Expand Up @@ -273,8 +272,9 @@ generate-swagger: $(SWAGGER_SPEC) ## generate the swagger spec from code comment

$(SWAGGER_SPEC): $(GO_SOURCES_NO_BINDATA)
$(GO) run $(SWAGGER_PACKAGE) generate spec -x "$(SWAGGER_EXCLUDE)" -o './$(SWAGGER_SPEC)'
$(SED_INPLACE) '$(SWAGGER_SPEC_S_TMPL)' './$(SWAGGER_SPEC)'
$(SED_INPLACE) $(SWAGGER_NEWLINE_COMMAND) './$(SWAGGER_SPEC)'
$(SED_INPLACE) -e '$(SWAGGER_SPEC_S_TMPL)' './$(SWAGGER_SPEC)'
$(SED_INPLACE) -e 's/\r$$//' './$(SWAGGER_SPEC)' # convert crlf to lf
$(SED_INPLACE) -e '$$a\' './$(SWAGGER_SPEC)' # add final newline
Comment on lines +276 to +277
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(SED_INPLACE) -e 's/\r$$//' './$(SWAGGER_SPEC)' # convert crlf to lf
$(SED_INPLACE) -e '$$a\' './$(SWAGGER_SPEC)' # add final newline

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this now as well, zero changes on my end

Copy link
Contributor

@wxiaoguang wxiaoguang Feb 20, 2025

Choose a reason for hiding this comment

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

Yes, no final new line is the expected result. Remove the no-sense final new line forever. See #33613 (comment)

just use the output by Go's json.MarshalIndent

Copy link
Contributor

@ChristopherHX ChristopherHX Feb 20, 2025

Choose a reason for hiding this comment

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

This worked at my first try, but no idk what this implies (crlf to lf not needed on my end)

Suggested change
$(SED_INPLACE) -e 's/\r$$//' './$(SWAGGER_SPEC)' # convert crlf to lf
$(SED_INPLACE) -e '$$a\' './$(SWAGGER_SPEC)' # add final newline
$(SED_INPLACE) -e 's/\r$$//' './$(SWAGGER_SPEC)' # convert crlf to lf
echo "" >> './$(SWAGGER_SPEC)' # add final newline

Now I wonder does this cause issues for others? (At least this is what I understand better as this is simple)

EDIT if we agree of removing final newline enforcement ignore this

Copy link
Contributor

Choose a reason for hiding this comment

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

This worked at my first try, but no idk what this implies (crlf to lf not needed on my end)

But I do not think the final new line makes sense here. The file content is just a generated JSON output, no one would really edit it.


.PHONY: swagger-check
swagger-check: generate-swagger
Expand All @@ -287,9 +287,9 @@ swagger-check: generate-swagger

.PHONY: swagger-validate
swagger-validate: ## check if the swagger spec is valid
$(SED_INPLACE) '$(SWAGGER_SPEC_S_JSON)' './$(SWAGGER_SPEC)'
$(SED_INPLACE) -e '$(SWAGGER_SPEC_S_JSON)' './$(SWAGGER_SPEC)'
$(GO) run $(SWAGGER_PACKAGE) validate './$(SWAGGER_SPEC)'
$(SED_INPLACE) '$(SWAGGER_SPEC_S_TMPL)' './$(SWAGGER_SPEC)'
$(SED_INPLACE) -e '$(SWAGGER_SPEC_S_TMPL)' './$(SWAGGER_SPEC)'

.PHONY: checks
checks: checks-frontend checks-backend ## run various consistency checks
Expand Down
Loading