google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.31k stars 210 forks source link

Disambiguate a resolve message #458

Closed filmil closed 1 year ago

filmil commented 1 year ago

I had a duplicate kwargs named 'disabled'. This ended up with a message 'keyword argument disabled repeated', which is really difficult to parse.

Hope to clarify things by changing that text a bit.

filmil commented 1 year ago

I would ask to keep as is.

This messaging helps in cases where different people added the same attribute in close proximity in time. It can be confusing if you see 'repeated', but you know you haven't repeated anything.

I just had a brief run-around because of this, and hence this change. The current wording could have helped.

adonovan commented 1 year ago

This messaging helps in cases where different people added the same attribute in close proximity in time. It can be confusing if you see 'repeated', but you know you haven't repeated anything.

Like all compiler errors, this is a static check of the code at a particular state. I don't really see how time (either execution time or codebase evolution) enters the picture. If a user sees this error, it means the string a= appears twice (i.e. is repeated) in the same function call. The error seems pretty clear to me.

filmil commented 1 year ago

Like all compiler errors, this is a static check of the code at a particular state. I don't really see how time (either execution time or codebase evolution) enters the picture. [...]

I understand. Happy to change to what is more acceptable to you. Upcoming.

The error seems pretty clear to me.

I understand that too.

I was noting down a user's perspective, where the message proved to be confusing. Again, happy to shorten the message.

filmil commented 1 year ago

Is done now. PTAL.