Skip to content

add quickindex table to std/meta.d#4376

Merged
dnadlinger merged 2 commits intodlang:masterfrom
wilzbach:patch-3
May 29, 2016
Merged

add quickindex table to std/meta.d#4376
dnadlinger merged 2 commits intodlang:masterfrom
wilzbach:patch-3

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented May 28, 2016

It seems like I am not the only one (e.g #4088) who didn't realize there was a system behind the naming in std.meta, better document it?

Ping @klickverbot

@dnadlinger
Copy link
Contributor

Shouldn't this rather go into the general Phobos style guide?

@wilzbach
Copy link
Contributor Author

Shouldn't this rather go into the general Phobos style guide?

I don't know, my idea was that I didn't immediately seeing a system behind:

Alias · AliasSeq · aliasSeqOf · allSatisfy · anySatisfy · ApplyLeft · ApplyRight · DerivedToFront · Erase · EraseAll · Filter · IndexOf · MostDerived · NoDuplicates · Repeat · Replace · ReplaceAll · Reverse · staticIndexOf · staticMap · staticSort · templateAnd · templateNot · templateOr

and a user is unlikely to read the style guide before reading the API?

An alternative idea would be to split the templates into categories as for std.traits?

@dnadlinger
Copy link
Contributor

Well, the other issue with adding that to the documentation here is that std.meta is a legacy module with inconsistent naming (even though it it has unfortunately be renamed only recently without addressing that). For example, there is no reason why staticMap should be lower-cased while Erase is not.

@dnadlinger
Copy link
Contributor

dnadlinger commented May 28, 2016

(There are also template{And, Or, Not}, where Andrei decreed that they be named inconsistently despite it being pointed out explicitly and Him having little public track record in writing that sort of code.)

@PetarKirov
Copy link
Member

PetarKirov commented May 28, 2016

An alternative idea would be to split the templates into categories as for std.traits?

Yeah, how about something like this:

Catergory Templates
Building blocks Alias AliasSeq aliasSeqOf
Alias sequence searching allSatisfy anySatisfy staticIndexOf
Alias sequence filtering Erase EraseAll Filter NoDuplicates
Alias sequence type hierarchy DerivedToFront MostDerived
Alias sequence transformation staticMap staticSort Replace ReplaceAll Reverse Repeat
Boolean operators templateAnd templateNot templateOr
Template instantiation ApplyLeft ApplyRight

(I left out IndexOf, as it should be deprecated.)

I think [staticMap, staticSort, templateAnd, templateNot, templateOr] should all be capitalized as they all return templates.

@wilzbach
Copy link
Contributor Author

I think [staticMap, staticSort, templateAnd, templateNot, templateOr] should all be capitalized as they all return templates.

Do you want to be the brave and propose the deprecation?
Btw the DStyle says the following, so hopefully we can convince Andrei.

Non-Eponymous Templates

The names of user-defined types should be PascalCased

Eponymous Templates

Templates which have the same name as a symbol within that template (and instantiations of that template are therefore replaced with that symbol) should be capitalized in the same way that that inner symbol would be capitalized if it weren't in a template - e.g. types should be PascalCased and values should be camelCased.

@PetarKirov
Copy link
Member

PetarKirov commented May 28, 2016

Do you want to be the brave and propose the deprecation?

I would, but I'm working on a Phobos proposal for an alternative to std.meta that also allows reusing a new form of ranges for sequence manipulation, and I don't want to spend time working on the "old" stuff :P
My proposal would make the alias sequence searching, filtering and transformation utilities in std.meta unnecessary.
I can't make any promises w.r.t. when my proposal will be ready, so anyone can pursue the deprecation process in the mean time.

@wilzbach wilzbach force-pushed the patch-3 branch 6 times, most recently from cd769da to 0fa8062 Compare May 29, 2016 02:26
@wilzbach
Copy link
Contributor Author

@ZombineDev I rebased this PR with your table.

Related PR to fix some table CSS: dlang/dlang.org#1315

(I left out IndexOf, as it should be deprecated.)

It was marked as "kept for backwards compatibilty" in May 2015.
See also: #3265

As it's already marked as in the documentation as "for backwards compatibilty", I added a deprecation warning to this PR.

My proposal would make the alias sequence searching, filtering and transformation utilities in std.meta unnecessary.

Looking forward to it!

@wilzbach wilzbach changed the title add note to explain naming in std/meta.d add quickindex table to std/meta.d May 29, 2016
@wilzbach wilzbach mentioned this pull request May 29, 2016
@PetarKirov
Copy link
Member

PetarKirov commented May 29, 2016

@ZombineDev I rebased this PR with your table.
I added a deprecation warning to this PR.

Thanks, obviously LGTM ;)

@MetaLang
Copy link
Member

Considering I embarrassingly missed the fact that there is already a std.meta.Filter and opened a PR to add it, I am all for a change that makes the names easier to see at a glance.

@wilzbach
Copy link
Contributor Author

Rebased so that issuing the deprecating warning for the deprecated IndexOf is a separate commit.

std/meta.d Outdated
* $(LREF Reverse)
* $(LREF Repeat)
* ))
* $(TR $(TD Boolean operators) $(TD
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the above paragraph introduces the term "template predicates" for what these templates operate on, the category should probably refer to it somehow.

@wilzbach
Copy link
Contributor Author

@klickverbot thanks for your feedback - I made a couple of changes

  1. renamed boolean operators to "Boolean template predicate operators"
  2. added allSatisfy and anySatisfy to this group
  3. created group "Alias sequence indexing" with staticIndexOf, and staticMap
  4. removed group "Alias sequence searching"
  5. sorted all groups alphabetically

@dnadlinger
Copy link
Contributor

We are sooo close to turning this into a giant collaborative bike-shed colouring event, but I still feel I should comment on the fact that staticMap is in the "indexing" category – why that? The connection to indexing gets quite tenuous when you consider that staticMap actually does the equivalent of flatMap in many functional languages because AliasSeqs are auto-expanding:

pragma(msg, staticMap!(ApplyLeft!(Repeat, 3), AliasSeq!(int, float)));

@wilzbach
Copy link
Contributor Author

We are sooo close to turning this into a giant collaborative bike-shed colouring event, but I still feel I should comment on the fact that staticMap is in the "indexing" category – why that?

It could be way worse. Don't be silly - your comment is very reasonable!
The sole reason is that staticIndexOf was lonely otherwise.
I moved staticMap to transformations and rebased ;-)

@dnadlinger
Copy link
Contributor

Auto-merge toggled on

@dnadlinger
Copy link
Contributor

Seems reasonable enough now to be a solid improvement on its own – if somebody disagrees, further tweaks can just be made as follow-up separate PRs.

@dnadlinger dnadlinger merged commit fdabc1d into dlang:master May 29, 2016
@PetarKirov
Copy link
Member

allSatisfy and anySatify are not predicte operators - they are just predicates.
I used the term "searching", because range algorithms with almost the same names and purpose are placed in std.algorithm.searching (all, any and countUntil). IMO, "searching" makes more sense and is more general than "indexing".

@wilzbach wilzbach deleted the patch-3 branch May 29, 2016 23:46
@wilzbach
Copy link
Contributor Author

@ZombineDev how about a quick follow-up pr?

@dnadlinger
Copy link
Contributor

@ZombineDev:

allSatisfy and anySatify […] are just predicates.

Not in the narrow sense in which the term is defined in this module (see the top-level comment). You are right that they don't transform predicates into predicates, though, they apply predicates to a sequence.

@PetarKirov
Copy link
Member

@klickverbot the term "template predicates" as defined in the documentation in this module does not apply to anything in this module. The only template that "takes a single argument" is Alias. I think the text was intended for std.traits's is* templates, but for some reason it was put here.

@PetarKirov
Copy link
Member

allSatisfy and anySatify can be considered N-ary predicates as they take N arguments and return a boolean value. It just so happens that the first argument is also a predicate (an unary) ;)

@PetarKirov
Copy link
Member

@ZombineDev how about a quick follow-up pr?

Alright, see PR #4382

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