Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Type formatting inconsistency #61

Open
ReedCopsey opened this issue May 19, 2016 · 19 comments
Open

Type formatting inconsistency #61

ReedCopsey opened this issue May 19, 2016 · 19 comments

Comments

@ReedCopsey
Copy link
Contributor

Throughout the site, types are formatted in two different ways. I thought I'd volunteer to correct this, but wanted to make sure it'd be accepted before I tackle the (large) set of changes.

For example, look at the ResizeArray<'T> type abbreviation page

The top of the page lists the type as T:System.Collections.Generic.List'1, but then code samples use the (more expected by end users) System.Collections.Generic.List<'T> style of formatting.

This is common - there's an extraneous T: prefix (I'm assuming from the original port of this data) that should be removed, but it would also be nice to refer to generic types via <'T> instead of '1 everywhere.

Would a PR be accepted which converts all type references to:

  1. Eliminate the T: prefix, and
  2. Use "nicer" formatted generic type descriptions

As I said, I'm happy to tackle this, but don't want to put in the effort if there's a reason to avoid either of these pieces. (It'd be easier to tackle them together, as the extra prefix provides a mechanism to find the other portion.)

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented May 19, 2016

We should use lowercase 'a.

We should also have a cannonical way to refer to an assembly member, statically checked, but this will come later I suppose; this would allow to make a lint checking that all members have a page, or that all links point to something actually existing in the assembly.

@ReedCopsey
Copy link
Contributor Author

If we go lower case, I'd prefer <'a>, since FSI does this:

> let foo value = ResizeArray<_>([ value ]);;

val foo : value:'a -> System.Collections.Generic.List<'a>

@ReedCopsey
Copy link
Contributor Author

Note that switching to <'a> / <'a,'b> might be more difficult to do globally in one pass - it would increase the scope significantly, though I'd happily do that for all of the files I edit, and try to find all other usages while I'm at it.

@smoothdeveloper
Copy link
Contributor

@ReedCopsey I think we should gear toward idiomatic F# more than reusing C# idioms in the parlance, we should for example say Foo seq rather than IEnumerable<Foo>, we also have to present F# in it's own context within the ML tradition rather than "just another .net language".

I understand you want to fix the inconsistency first, maybe we can share the load after your first commit, we can split the work to review / adjust to more idiomatic if necessary.

@dsyme
Copy link
Contributor

dsyme commented May 20, 2016

@smoothdeveloper Here are the guidelines:

  • For all F# code it is idiomatic to use uppercase type variable names 'T etc. Lower case is only used in the output of type inferred functions. See Do use PascalCase for generic parameter names in the F# Component Guidelines

  • The following are preferred for writing types, see the component guidelines

    seq<Foo> (not Foo seq or IEnumerable<Foo>)
    Foo[] (not Foo array)
    Foo list (though list<Foo> is ok as well, if preferred for consistency)
    Set<Foo>
    Map<int,Foo>
    ResizeArray<Foo> (instead of System.Collections.Generic.List<Foo>)

  • For other cases see the component guidelines

This is how it's always been since F# 2.0 and we must stick to it.

Thanks
Don

@smoothdeveloper
Copy link
Contributor

@dsyme, thanks for reference to guidelines and quoting the relevant bits!

Ok, we should stick to that for editing documentation, @ReedCopsey whenever you'll want another pair of eye for review or contribution on your first pass, please let me know.

For the trivia (and won't be too curious going farther) but who made cup<'t> and why wasn't it following guidelines? 't cup feels the most elegant to me.

More seriously, should I be worried that sometype option and others might not be supported by compiler? I think that's a major pro for readability to have those (same for seq, array, list and others) instead of the usual and verbose "Angle Brackets" among all but few languages, and I'd like to stick to this for the internals of my code.

@dsyme
Copy link
Contributor

dsyme commented May 20, 2016

For the trivia (and won't be too curious going farther) but who made cup<'t> and why wasn't it following guidelines?

Where do you see that cup? It's wrong! Both the type name and the generic type parameter name!

@ReedCopsey
Copy link
Contributor Author

@dsyme sent to contributors by Visual F# team

@smoothdeveloper
Copy link
Contributor

@dsyme (when we can't bike-shed operator location in compiler 😄 ) https://blogs.msdn.microsoft.com/dotnet/2015/07/02/thank-you-for-your-contributions/

For me SHOUTING_CODE is wrong, be it sql, type parameters in .net generics, java erased generics etc.

It seems those bits which ended in F# Component Guidelines is what justifies the most "Influenced by ... C#, ..." I've seen somewhere about F#!

@ReedCopsey
Copy link
Contributor Author

So - My proposal would be for me to go through and:

  1. Remove the T: prefixes. I suspect these were from automatic XML doc comment conversions, and really are "wrong"
  2. Change generic type references everywhere I see them, while at it, to using Foo<'T>, Foo<'T1,'T2>, or Foo<'T1,'T2,'U> styling (this matches usages in existing code in samples, from what I can tell -where 'T/'U is used, but 'T1,'T2,'U is preferred if it's something with inputs and output types, such as references to System.Func or similar.

@dend : Does this seem reasonable? Again, I wanted confirmation that this would be accepted prior to putting forward that much effort.

@dend
Copy link
Contributor

dend commented May 20, 2016

@ReedCopsey seems reasonable. I will be monitoring inbound PR and will add comments if something's off.

@ReedCopsey
Copy link
Contributor Author

@dend I'll start working on it - are you okay if I do it in stages? From my initial count, this will impact at least 363 files - might make sense to do them in batches

@dend
Copy link
Contributor

dend commented May 20, 2016

@ReedCopsey of course, docs are always in flow.

@KevinRansom
Copy link
Contributor

@dsyme @ReedCopsey Yeah cup of <'t> doesn't follow the guidelines, but then neither does about 50% of the code I've seen. When we made the cup we weren't actually aware of the guideline, Don pointed it out after we hade them made.

Personally I prefer lower case, the less upper case, camel case, and Pascal case in the source code the better I like it. However I don't get to decide these things.

Kevin

@ReedCopsey
Copy link
Contributor Author

Note that, while working on this, I also discovered:

P: on properties references
M: on method references
exceptions tag errors when exception references exist.

Adding these to things to cleanup.

[Also cleaning up code samples while editing these docs - many are in rough shape, like the main array page.]

@smoothdeveloper
Copy link
Contributor

First things first:

@ReedCopsey

1 Remove the T: prefixes. I suspect these were from automatic XML doc comment conversions, and really are "wrong"
2 Change generic type references everywhere I see them

It seems important that we do those things in a way which will enable easily to update yet again those references to point to the actual msdn reference page for those types, what is the process?

For the specifics of T,U or T,T2 etc. I guess we need @dsyme input (@dend/@dsyme can we put that in a rough list in readme.md? it will impact many contributions).

I suspect these were from automatic XML doc comment conversions

@dend @dsyme @KevinRansom can we get a bit more background of all those things we will need to suspect / infer / guess and a rough idea of where technically this will be going (at least to have everything in outstanding shape under msdn)? Maybe redacting something in the readme.md of this repository or in wiki pages would help the "long running contributors" to make the long-haul work more efficient?

I've made small suggestions in #66 #68 to help everyone have better feedback loop, it would rock to have this addressed soon 😄

@KevinRansom I'm surprised you don't like NuGet.exe but I appreciate that for bikeshedding (although casing, filesystem and xplat are not to be qualified bikeshedding) we seem to have same tastes 😄

Anyways, it's great to have all that documentation on github, another time at MS, F# strikes first to change processes :)

@smoothdeveloper
Copy link
Contributor

It seems important that we do those things in a way which will enable easily to update yet again those references to point to the actual msdn reference page for those types, what is the process?

Thinking for myself on that, maybe we can have an index of all symbols (based on C# nameof) in all libraries, and perform substitution against fully qualified names appearing inline in the textual contents?

It could be computationally expensive to process, but I've heard about Azure (and MS has an account there) and Mbrace, this could be a good show-case 😄

I somehow believe, in absence of anything better, this could be effective to get links processed without spending time formatting each bit of reference in the source files.

@dend
Copy link
Contributor

dend commented May 20, 2016

I have #57 open and assigned to myself regarding guidelines and help.

@ReedCopsey
Copy link
Contributor Author

I'll also remove "Overload:" prefixes while I do these cleanups.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants