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

<ranges>: rename views::iota and views::repeat parameters #4908

Merged
merged 1 commit into from
Aug 25, 2024

Conversation

SuperWig
Copy link
Contributor

The Visual Studio IDE has an "inline parameter hint" feature which will show the parameter names inline. The STL generally has nice descriptive names which can make this feature helpful.
When I went to use views::iota I forgot if the second arg is a count or not, I was hoping the parameter names would save me a trip to cppreference, but alas, I was greeted with useless names.

views::repeat has the same issue (which is my fault; I just copied and pasted iota)

image

I also have a feature request to de-uglify them (might make another to not show the type information for these views, or I guess more generally, long nested templated types)

@SuperWig SuperWig requested a review from a team as a code owner August 24, 2024 13:51
@StephanTLavavej StephanTLavavej added enhancement Something can be improved ranges C++20/23 ranges labels Aug 24, 2024
@StephanTLavavej
Copy link
Member

Thanks! 😻 This is a strict improvement over the status quo's currently terrible names, so I'm happy here.

I observe that this isn't perfectly consistent with either the underlying views' parameters, or the Standard-depicted parameters, but I don't care enough to nitpick it further. (The views are the preferred interface for users, and the names you've chosen are a bit more descriptive than "value, bound", which is enough rationale for me.)

I'll defer to Casey if he has other thoughts.

@SuperWig
Copy link
Contributor Author

I observe that this isn't perfectly consistent with either the underlying views' parameters, or the Standard-depicted parameters, but I don't care enough to nitpick it further. (The views are the preferred interface for users, and the names you've chosen are a bit more descriptive than "value, bound", which is enough rationale for me.)

After I made the commit I realised the same thing... I then also came to pretty much the same conclusion so opted to just create the PR anyway 😁.

@CaseyCarter
Copy link
Member

views::repeat has the same issue (which is my fault; I just copied and pasted iota)

Hey, at least we didn't name them (_Val2, _Val1).

@CaseyCarter CaseyCarter removed their assignment Aug 24, 2024
@StephanTLavavej StephanTLavavej self-assigned this Aug 25, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 8af2cc4 into microsoft:main Aug 25, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

_Thanks1, _Thanks2 😹 🤪 🎉

@SuperWig SuperWig deleted the Prams branch August 26, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved ranges C++20/23 ranges
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants