toml icon indicating copy to clipboard operation
toml copied to clipboard

v0.9 no longer displays the name of the duplicate key and table on parse

Open zanieb opened this issue 5 months ago • 2 comments

See https://github.com/zanieb/toml-regression-1 for a simple reproduction

In 0.8.19

TOML parse error at line 4, column 1
  |
4 | python-multipart = "https://example.com/bar.whl"
  | ^
duplicate key `python-multipart` in table `tool.uv.sources`

In 0.9.2

TOML parse error at line 4, column 1
  |
4 | python-multipart = "https://example.com/bar.whl"
  | ^^^^^^^^^^^^^^^^
duplicate key

It seems okay to avoid repeating python-multipart but the "table ..." part seems pretty helpful.

Originally discovered at https://github.com/astral-sh/uv/pull/14571#discussion_r2213787884

zanieb avatar Jul 21 '25 13:07 zanieb

The duplicate key was dropped because at that time, the parser didn't support allocations for errors and I felt that being able to show the context would make up for it enough that this didn't motivate adding allocations.

Showing which table it was in felt redundant (since we show the location). If the case can be made for it, it will require expanding our error reporting scheme as it doesn't support that today. What it does support is the ability to highlight other sections in the file. We aren't currently exposing this or using it in our own rendering. In some cases, what the parser is providing the span for is the location of the first key that the current key is a duplicate of.

epage avatar Jul 21 '25 17:07 epage

Makes sense, I figured it was motivated by changes like that :)

Showing which table it was in felt redundant (since we show the location)

While it's technically redundant, I think it's less obvious where to go from just the line numbers. I think it's hard to argue it's not more user friendly to display the table name in some way, though I think it's fair to argue it's not worth the effort.

What it does support is the ability to highlight other sections in the file.

That could definitely be a nice approach too, though I think it can be hard to get right.

No pressure to make any changes here, just wanted to make sure you were aware these changed.

zanieb avatar Jul 21 '25 17:07 zanieb