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

Error while getting .editorconfig file #20694

Closed
somera opened this issue Aug 6, 2022 · 19 comments · Fixed by #21257
Closed

Error while getting .editorconfig file #20694

somera opened this issue Aug 6, 2022 · 19 comments · Fixed by #21257
Labels
type/question Issue needs no code to be fixed, only a description on how to fix it yourself.

Comments

@somera
Copy link

somera commented Aug 6, 2022

Description

I found this message in /admin/notices

Error while getting .editorconfig file: normalization error: insert_final_newline=off is not an acceptable value. strconv.ParseBool: parsing "off": invalid syntax

Nothing founded in gitea.log.

Gitea Version

1.17.0

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

image

Git Version

2.25.1

Operating System

Ubuntu 20.04.4

How are you running Gitea?

Downloaded gitea-1.17.0-linux-amd64

Database

PostgreSQL

@somera somera added the type/bug label Aug 6, 2022
@silverwind
Copy link
Member

It should be true, false or unset. Best course of action is to fix the .editorconfig file. I guess it's good to see such errors somewhere instead of silently ignoring them, so I'd say it's working as intended.

@somera
Copy link
Author

somera commented Aug 7, 2022

Is this an gitea internal error? Error from the update mirror task?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 8, 2022

It's reported by "editorconfig.Parse", which is called by SetEditorconfigIfExists when you browses the repo commits.

The problem is that you have invalid syntax in your .editorconfig

@wxiaoguang wxiaoguang added type/question Issue needs no code to be fixed, only a description on how to fix it yourself. and removed type/bug labels Aug 8, 2022
@somera
Copy link
Author

somera commented Aug 8, 2022

It's reported by "editorconfig.Parse", which is called by SetEditorconfigIfExists when you browses the repo commits.

The problem is that you have invalid syntax in you .editorconfig

I found this: https://github.com/go-gitea/gitea/blob/main/.editorconfig

But in my gitea setup I can't find any .editorconfig.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 8, 2022

It's in your repository. It's a user file, committed by some user into your repository.

@somera
Copy link
Author

somera commented Aug 8, 2022

Its in your repository. Its a user file, committed by some user into your repository.

Ach o.k. That must be one of the mirrors. In this case I can't fix it.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 8, 2022

So maybe the proposal could be: "Make Gitea not report user's editorconfig syntax error as repository notice", or only show the syntax error on the proper page clearly.

@silverwind
Copy link
Member

silverwind commented Aug 8, 2022

We should probably treat values with such errors the same as unset. Maybe also show it in the repo/file view, but I think it's generall unnecessary.

@Gusted
Copy link
Contributor

Gusted commented Aug 9, 2022

We should probably treat values with such errors the same as unset. Maybe also show it in the repo/file view, but I think it's generall unnecessary.

Well, we can ignore the error, but you won't receive the editoconfig anymore then. As these are "hard" errors, https://github.com/editorconfig/editorconfig-core-go/blob/560dd96cabfee7b13fc1e3a3cbcb23c2fcf7258b/definition.go#L53 and will just return nil as editorconfig, this library doesn't seem to have any option to change/disable this behavior either :/

@silverwind
Copy link
Member

Best course of action would probably be to make editorconfig-core-go treat directive parsing errors the same as when the directive is absent. Could also have a editorconfig.ParseGraceful for that behaviour. Maybe @greut has an opinion.

@greut
Copy link
Contributor

greut commented Aug 9, 2022

it should be easy to handle either on/off. Are those part of the spec? A graceful parsing is probably much more work...

@silverwind
Copy link
Member

silverwind commented Aug 9, 2022

No, the spec only indicates true, false and unset as valid values. It does suggest graceful parsing, though:

EditorConfig plugins shall ignore unrecognized keys and invalid/unsupported values for those keys.

So while I think supporting boolean aliases like YAML does (yes, no, true, false, on, off) is probably fine, I think ultimately, invalid values should just be ignored instead of raising errors.

@greut
Copy link
Contributor

greut commented Aug 9, 2022

@silverwind agreed. Most of them, could be moved to an error as log.

@silverwind

This comment was marked as off-topic.

@silverwind
Copy link
Member

Hmm, logging these parser "warnings" might be hard to consume. How about adding a warnings slice to a parse result, e.g.?

type Editorconfig struct {
  Root        bool
  Definitions []*Definition
  config      *Config
  warnings    []string
}

Fatal errors should still return err of course.

@lunny
Copy link
Member

lunny commented Mar 29, 2023

Is this still a problem?

@silverwind
Copy link
Member

Yes, #21257 got stalled. I think it's the right solution to parse graceful and display errors. Ideally the admin page could also show the affected repo, or the admin notice could also be removed entirely.

jolheiser pushed a commit that referenced this issue Apr 6, 2023
…med editorconfigs (#21257)

The _graceful_ should fail less when the `.editorconfig` file isn't
properly written, e.g. boolean values from YAML or unparseable numbers
(when a number is expected). As is... information is lost as the
_warning_ (a go-multierror.Error) is ignored. If anybody knows how to
send them to the UI as warning; any help is appreciated.

Closes #20694

Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
@somera
Copy link
Author

somera commented Apr 15, 2023

Should this be "fixed" with 1.19.1? Cause

image

@lunny
Copy link
Member

lunny commented Apr 16, 2023

Not yet. It's only for 1.20

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/question Issue needs no code to be fixed, only a description on how to fix it yourself.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants