Skip to content

[Merged by Bors] - fix: improve line-wrapping of (D)Finsupp reprs#17882

Closed
eric-wieser wants to merge 2 commits intomasterfrom
eric-wieser/finsupp-notation
Closed

[Merged by Bors] - fix: improve line-wrapping of (D)Finsupp reprs#17882
eric-wieser wants to merge 2 commits intomasterfrom
eric-wieser/finsupp-notation

Conversation

@eric-wieser
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser commented Oct 17, 2024

These were previously wrapping very poorly.

Also update tests to use #guard_msgs more effectively, and fix a typo in the delaborators for DFinsupp.


Open in Gitpod

Also update tests to use guard_msgs more effectively.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 17, 2024

PR summary 54230edc2e

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

/--
info:
-/
/-- info: (DFinsupp.single 1 3).update 2 3 : Π₀ (i : ℕ), Fin (i + 10) -/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this get broken? Before it would say fun₀ | 1 => 3 | 2 => 3.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I accidentally replaced eval with check. I guess check was already broken.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +24 to +26
| ["there are five words here", "and five more words here"] => 5
| ["there are seven words but only here"] => 7
| ["just two"] => 2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation doesn't match between the #eval and the #check command, can you fix that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version do you prefer?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the two-space-indented version is a bit clearer, but either works for me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@Vierkantor Vierkantor added the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 18, 2024
@eric-wieser eric-wieser removed the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 18, 2024
Copy link
Copy Markdown
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

bors r+

@ghost ghost added the ready-to-merge This PR has been sent to bors. label Oct 18, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 18, 2024
These were previously wrapping very poorly.

Also update tests to use `#guard_msgs` more effectively, and fix a typo in the delaborators for DFinsupp.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Oct 18, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix: improve line-wrapping of (D)Finsupp reprs [Merged by Bors] - fix: improve line-wrapping of (D)Finsupp reprs Oct 18, 2024
@mathlib-bors mathlib-bors bot closed this Oct 18, 2024
@mathlib-bors mathlib-bors bot deleted the eric-wieser/finsupp-notation branch October 18, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-data Data (lists, quotients, numbers, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants