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

std: Rename Iterator adaptor types to drop the -Iterator suffix #8090

Closed
wants to merge 1 commit into from
Closed

std: Rename Iterator adaptor types to drop the -Iterator suffix #8090

wants to merge 1 commit into from

Conversation

bluss
Copy link
Member

@bluss bluss commented Jul 28, 2013

Drop the "Iterator" suffix for the the structs in std::iterator.
Filter, Zip, Chain etc. are shorter type names for when iterator
pipelines need their types written out in full in return value types, so
it's easier to read and write. the iterator module already forms enough
namespace.

@bluss
Copy link
Member Author

bluss commented Jul 28, 2013

ok thestinger hinted that it might be suboptimal to have structs named Map, Skip, SkipWhile etc. These struct names all correspond directly to the iterator adaptor method name (transform being named .map in spirit), so I still prefer them.

@alexcrichton
Copy link
Member

I'm actually super in-favor of naming schemes like this. In my opinion this is one of the things which I believe go did really well. It's different for them in that it's impossible to import a name into a package, you can only import package names into packages. That way in go when referencing types it's package.Type which if you're making an iterator name like iterator.Iterator, it seems a bit redundant.

I'm personally in favor of this change, but rust is different in that it is possible to import iterator::Map into the current namespace. I also think that this isn't necessarily the best style because the whole point of short names is that the package name + the short name is what makes sense, not just the type name.

For this patch in particular, I would be in favor of removing any use iterator::{XXX, XXX, XXX} imports and changing them to use iterator; and then referencing all types through the iterator module. That way everything is still clear what it's actually intending (iterator::Map is far less ambiguous than Map).

I'm hesitant to r+ this though because those are my own opinions and there's probably contention over this, so what are others' thoughts?

@thestinger
Copy link
Contributor

@alexcrichton: It's reasonable to have a type named Chain and a corresponding chain method, or Enumerator and enumerate. It's very strange to have types given verbs as names though, like SkipWhile.

When I originally started naming these I considered coming up with shorter names for most, but ended up just using an Iterator suffix for consistency.

@alexcrichton
Copy link
Member

I do agree that we should definitely pick one. We should either use the Iterator suffix everywhere, or we shouldn't. Things like iterator::SkipWhile make just as much sense to me as SkipWhileIterator in my opinion though. That's the unfortunate part in that if you reference just SkipWhile as a type it doesn't make any sense, it must be reference through iterator::SkipWhile to make any sense really.

@bluss
Copy link
Member Author

bluss commented Jul 28, 2013

I prefer exact correspondance, .skip_while() to SkipWhile (not trying to nounise it to SkipWhiler, WhileSkipper haha). Also the user code decides if they want to use the namespace iterator::SkipWhile or not. I think it's fine to stick to importing them by name -- half the point is to have more concise function signatures.

Drop the "Iterator" suffix for the the structs in std::iterator.
Filter, Zip, Chain etc. are shorter type names for when iterator
pipelines need their types written out in full in return value types, so
it's easier to read and write. the iterator module already forms enough
namespace.
@bluss
Copy link
Member Author

bluss commented Jul 29, 2013

rebased only to resolve conflicts

bors added a commit that referenced this pull request Jul 29, 2013
Drop the "Iterator" suffix for the the structs in std::iterator.
Filter, Zip, Chain etc. are shorter type names for when iterator
pipelines need their types written out in full in return value types, so
it's easier to read and write. the iterator module already forms enough
namespace.
@bors bors closed this Jul 30, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 30, 2021
Fix `enum_variants` FP on prefixes that are not camel-case

closes rust-lang#8090

Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.

This changes how the lint behaves:

1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
    cFoo,
    cBar,
    cBaz,
}

enum Something {
    CCall,
    CCreate,
    CCryogenize,
}
```

2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
    PrefixXXX,
    PrefixTea,
    PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.

changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.

---

 (Edited by `@xFrednet` removed some non ascii characters)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants