leanprover / leanprover/lean4

RFC: revisit error ranges for `by`, `.`, `next`, `case`

Open
#8,919 3 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

P-medium RFC RFC accepted
Dominant language
Lean
Stars
9.2k
Forks
990
Avg merge
1d 17h
Merged PRs (30d)
175

Description

I'd like to revisit the error range spans for proof block starting syntax (by, ., next, case).

This is the status quo:

Image

And it is justified as follows:

/--
  Use position of `=> $body` for error messages.
  If there is a line break before `body`, the message will be displayed on `=>` only,
  but the "full range" for the info view will still include `body`. -/

(Until I fixed that in #4528, the dot had the error only on the dot itself.)

I understand that reasoning, but I believe since the introduction of the “unsolved goals” marker and in particular the the gutter decorations in the vscode plugin (https://github.com/leanprover/vscode-lean4/pull/585) the facts have changed: There are now two other visual indicators that make the range visible.

The issue I’d like to improve here is that even I, as arguably an experience lean user, tend to take a few moments to distinguish between “the first tactic has an error” and “the proof block is incomplete”. I assume that for beginners this must be even worse.

Also new features like simp warning on unused arguments (#8901) mean that more tactics would like to make use of the “squiggly line real estate” and get their signals drowned if they happen to be in the first line of a block.

So the proposal here is put the error range on the keyword only (case, next, by, .). Putting this on the first keyword (and not the =>) makes it also visually uniform and (IMHO) easier to grasp.

Of course the fullrange should still include the whole tactic block.

One downside is that users that are using editors that don’t support the gutter decoration have smaller squiggly errors. I’d say we should optimize for vscode, and it’s not that they are completely missing the signal, it’s just less visible (as it was before #4528). And of course they also benefit from not seeing red squiggly lines under an actually successful tactic.

Impact

Add 👍 to issues you consider important. If others benefit from the changes in this proposal being added, please ask them to add 👍 to it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the proof-block handling for by, ., next, and case, then review the range behavior discussed in #4528 and the gutter-decoration context in vscode-lean4 PR #585. Done means diagnostic ranges cover only the keyword while fullrange still covers the entire tactic block, with the editor tradeoff resolved.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.