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

Indent body and type when printing local definitions over several lines #18928

Merged
merged 1 commit into from Apr 16, 2024

Conversation

herbelin
Copy link
Member

@herbelin herbelin commented Apr 12, 2024

Assume:

Theorem T (long_long_long_long_long_long_long_long_ident := fun x : nat => x) : True.

Before the PR, it is printed

  long_long_long_long_long_long_long_long_ident :=
  fun x : nat => x : nat -> nat
  ============================
  True

After, it is printed:

  long_long_long_long_long_long_long_long_ident :=
    fun x : nat => x : nat -> nat
  ============================
  True

The PR also applies this indentation to other similar places.

Overlays (to be merged before the current PR)

@herbelin herbelin added kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. part: printer The printing mechanism of Coq. labels Apr 12, 2024
@herbelin herbelin added this to the 8.20+rc1 milestone Apr 12, 2024
@herbelin herbelin requested a review from a team as a code owner April 12, 2024 08:22
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Apr 12, 2024
@proux01
Copy link
Contributor

proux01 commented Apr 12, 2024

@herbelin apparently this requires modifying a few output tests

@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Apr 12, 2024
@herbelin herbelin force-pushed the master+indent-long-decls-printing branch from 6ef96ff to 8cfba2a Compare April 12, 2024 11:59
@coqbot-app coqbot-app bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels Apr 12, 2024
@herbelin
Copy link
Member Author

apparently this requires modifying a few output tests

OK. I seized then the opportunity to also "improve" the printing of multiple names of same type.

@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Apr 12, 2024
@herbelin herbelin force-pushed the master+indent-long-decls-printing branch from 8cfba2a to 2cb8e09 Compare April 12, 2024 18:14
@coqbot-app coqbot-app bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Apr 12, 2024
printing/printer.ml Outdated Show resolved Hide resolved
printing/printer.ml Outdated Show resolved Hide resolved
@proux01 proux01 force-pushed the master+indent-long-decls-printing branch from 2cb8e09 to c3e41f0 Compare April 14, 2024 09:35
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Apr 14, 2024
For instance:

Theorem T (long_long_long_long_long_long_long_long_ident := fun x : nat => x) : True.

Before:

  long_long_long_long_long_long_long_long_ident :=
  fun x : nat => x : nat -> nat
  ============================
  True

After:

  long_long_long_long_long_long_long_long_ident :=
    fun x : nat => x : nat -> nat
  ============================
  True

At the same time, we:
- improve the printing of multiple variables with same time (see test-suite)
@proux01 proux01 force-pushed the master+indent-long-decls-printing branch from c3e41f0 to 55fc795 Compare April 15, 2024 14:10
@proux01
Copy link
Contributor

proux01 commented Apr 15, 2024

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Apr 15, 2024
@SkySkimmer
Copy link
Contributor

@proux01 are you going to self assign?

Looks like the iris output tests are sensitive to this change, cc @RalfJung, @robbertkrebbers

@proux01 proux01 self-assigned this Apr 16, 2024
@RalfJung
Copy link
Contributor

The Iris issue should be fixed now, sorry for the inconvenience.

@proux01
Copy link
Contributor

proux01 commented Apr 16, 2024

Thanks a lot

@proux01
Copy link
Contributor

proux01 commented Apr 16, 2024

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 56ed9dd into coq:master Apr 16, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. part: printer The printing mechanism of Coq.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants