Error while getting .editorconfig file #9359

@wxiaoguang commented on GitHub (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.

@wxiaoguang commented on GitHub (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.

GiteaMirror commented

@silverwind commented on GitHub (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.

@silverwind commented on GitHub (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.

GiteaMirror commented

@Gusted commented on GitHub (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, 560dd96cab/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 :/

@Gusted commented on GitHub (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 :/

GiteaMirror commented

@silverwind commented on GitHub (Aug 9, 2022):

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.

@silverwind commented on GitHub (Aug 9, 2022): 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.

GiteaMirror commented

@greut commented on GitHub (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...

@greut commented on GitHub (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...

GiteaMirror commented

@silverwind commented on GitHub (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.

@silverwind commented on GitHub (Aug 9, 2022): No, [the spec](https://editorconfig-specification.readthedocs.io/) 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.

GiteaMirror commented

@greut commented on GitHub (Aug 9, 2022):

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

@greut commented on GitHub (Aug 9, 2022): @silverwind agreed. Most of them, could be moved to an error as log.

GiteaMirror commented

@silverwind commented on GitHub (Aug 9, 2022):

On a related note, according to https://stackoverflow.com/a/57682840/808699, YAML 1.2 dropped the yes/no on/off aliases, so I think it's probably wise to follow and be strict about true/false/unset, which is also in-line with the spec.

@silverwind commented on GitHub (Aug 9, 2022): On a related note, according to https://stackoverflow.com/a/57682840/808699, YAML 1.2 dropped the yes/no on/off aliases, so I think it's probably wise to follow and be strict about true/false/unset, which is also in-line with the spec.

GiteaMirror commented

@silverwind commented on GitHub (Aug 9, 2022):

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.

@silverwind commented on GitHub (Aug 9, 2022): Hmm, logging these parser "warnings" might be hard to consume. How about adding a `warnings` slice to a parse result, e.g.? ```go type Editorconfig struct { Root bool Definitions []*Definition config *Config warnings []string } ``` Fatal errors should still return `err` of course.

GiteaMirror commented

@lunny commented on GitHub (Mar 29, 2023):

Is this still a problem?

@lunny commented on GitHub (Mar 29, 2023): Is this still a problem?

GiteaMirror commented

@silverwind commented on GitHub (Mar 29, 2023):

Yes, https://github.com/go-gitea/gitea/pull/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.

@silverwind commented on GitHub (Mar 29, 2023): Yes, https://github.com/go-gitea/gitea/pull/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.

GiteaMirror commented

@somera commented on GitHub (Apr 15, 2023):

Should this be "fixed" with 1.19.1? Cause

@somera commented on GitHub (Apr 15, 2023): Should this be "fixed" with 1.19.1? Cause ![image](https://user-images.githubusercontent.com/8334250/232239480-36c5c804-925f-4091-9c01-2a5ae8aefc59.png)

GiteaMirror commented