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

[Merged by Bors] - feat: list lemmas #11626

Closed
wants to merge 17 commits into from
Closed

[Merged by Bors] - feat: list lemmas #11626

wants to merge 17 commits into from

Conversation

sven-manthe
Copy link
Collaborator

@sven-manthe sven-manthe commented Mar 24, 2024

Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.


These lemmas are used in a project on Borel determinacy that may be added to mathlib.

Open in Gitpod

@sven-manthe sven-manthe added awaiting-review The author would like community review of the PR new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! labels Mar 24, 2024
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and welcome to mathlib!

I gave your PR a first look and have a lot of style comments. (This is perfectly normed for a first PR.) Quite a few things are also copied from the style guide; did you get to read that already?

Some issues occur several times - I haven't mentioned them all the time; please also address similar instances below.

Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
@grunweg grunweg added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Mar 25, 2024
@sven-manthe
Copy link
Collaborator Author

Thank you. I had a look at the style guide already, but my attempts to fix this with a few regexps didn't find all the issues. Now, apart from "hypotheses left of colon" (see above), I tried to fix all the mentioned issues

@sven-manthe sven-manthe added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Mar 26, 2024
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks! You're right, the style guide doesn't mention spaces between tactics and their arguments. I think that's an oversight and should be fixed. (And I'll file a PR to remove the other occurrences.)

Just a few more, then I'm out of comments :-)

Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
@grunweg
Copy link
Collaborator

grunweg commented Mar 26, 2024

One more thing: when you're addressed a comment, can you click "resolve conversation"? (If you would still like some opinion on a question, just leave it open. But right now, it's hard to see which is which.) Thanks!

Update: I just did this for most of them.

@grunweg grunweg added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Mar 26, 2024
@sven-manthe
Copy link
Collaborator Author

(And I'll file a PR to remove the other occurrences.)
Note that I fixed the occurences in this file already in the last commit (to avoid merge conflicts)

@sven-manthe sven-manthe added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Mar 26, 2024
@grunweg
Copy link
Collaborator

grunweg commented Mar 26, 2024

(And I'll file a PR to remove the other occurrences.)
Note that I fixed the occurences in this file already in the last commit (to avoid merge conflicts)

I know! For me, it was easier to fix them all - I added you as a co-author on #11714. (Feel free to tweak how you're mentioned in the PR description.) I hope that's also fine!

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Mar 26, 2024
Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. I still have a few comments.

Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
@fpvandoorn fpvandoorn added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Mar 29, 2024
@fpvandoorn
Copy link
Member

Also, please merge master because there is a merge conflict. Let me know if you need help / need me to do it.

suggested changes, added ext_iff
@sven-manthe sven-manthe added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes merge-conflict The PR has a merge conflict with master, and needs manual merging. labels Mar 29, 2024
sven-manthe and others added 4 commits March 29, 2024 16:43
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@sven-manthe sven-manthe added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Mar 29, 2024
Mathlib/Data/List/Basic.lean Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
Mathlib/Data/List/Infix.lean Outdated Show resolved Hide resolved
sven-manthe and others added 3 commits March 29, 2024 18:55
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
sven-manthe and others added 3 commits March 29, 2024 19:36
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

LGTM
bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 4, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 4, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 4, 2024

Build failed (retrying...):

mathlib-bors bot pushed a commit that referenced this pull request Apr 4, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 4, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: list lemmas [Merged by Bors] - feat: list lemmas Apr 4, 2024
@mathlib-bors mathlib-bors bot closed this Apr 4, 2024
@mathlib-bors mathlib-bors bot deleted the sven-list-1 branch April 4, 2024 15:26
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Add some general purpose lemmas on lists. The new ext_get?' is intermediate between ext and ext_get, and for consistent naming add an alias ext_get? for ext.



Co-authored-by: sven-manthe <147848313+sven-manthe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants