dart-lang / dartdoc

API documentation tool for Dart.
https://pub.dev/packages/dartdoc
BSD 3-Clause "New" or "Revised" License
473 stars 118 forks source link

dartdoc can generate broken docs on case-insensitive filesystems #1196

Open chalin opened 8 years ago

chalin commented 8 years ago

The class NgIf in angular2.common also has a member named ngIf. Notice that these names differ only because of capitalization.

Currently, in the generated angular2.common/NgIf-class.html selecting the "details" links for the class and the member sends us to, respectively:

as expected, but the content of both of these pages is the same (they both describe the class member, not the class).

/cc @kwalrath

keertip commented 8 years ago

@chalin , could you add instructions on how to generate the docs. Thanks!

chalin commented 8 years ago

Surely. I've updated the the comment over https://github.com/angular/angular.io/pull/1888 with more detailed instructions about how to install, patch, build and serve the docs. Let me know if you have more questions.

devoncarew commented 8 years ago

From looking at this, the issue is that the macos filesystem preserves case in filenames but is case insensitive. We write the docs for the constructor Ngif.Ngif(), then the docs for the member Ngif.ngif. The last write to Ngif/ngif.html wins.

This is a difficult issue for us to workaround since we rely on Dart namespaces when we generate file names. We could disambiguate at generation time, but having nice, simple, predictable file names for dart symbols make it much easier for other tools to generate links to hosted dart docs.

chalin commented 8 years ago

A simple solution is to generate the docs under linux. @naomiblack, @kwalrath : is that realistic (since you handle the doc generation just before deployment)?

kwalrath commented 8 years ago

I'm not sure it's realistic. It'd be inconvenient, at the very least, since I don't normally use a Linux box.

chalin commented 8 years ago

We could disambiguate at generation time, but having nice, simple, predictable file names for dart symbols make it much easier for other tools to generate links to hosted dart docs.

Agreed. There are disambiguation rules in place now that are "nice, simple, and predictable". Maybe they can be tweaked just a bit.

Here is an example of the current disambiguation that is in place. Consider the compilation unit:

class Main {}
void main() {}

The base name for the doc entries would be: Main-class and main. The disambiguation helps avoid potential clashes between classes and other globally declared library members whose name differ only in case.

Why not qualify the doc file base name for constructors? That would keep the rules simple, and help solve our current problem. E.g., something like NgIf-constructor vs. ngIf (for the class member of the same name).

If we don't want to do that for all constructors, then maybe it can be done only for the "default" constructor --- i.e., the constructor with a simple (unqualified) name that matches the name of the class (vs. constructors with qualified names like Future.value()). I'm not sure that it is worth the extra complication in the rules though.

Thoughts?

kwalrath commented 8 years ago

@devoncarew what do you think of Patrice's proposal?

jcollins-g commented 7 years ago

This has implications on filesystems which are case-insensitive (Windows/Mac, by default).

chalin commented 7 years ago

Yes, the issue arose because @kwalrath and I mainly build under MacOS. What do you think of the proposal given a few comments above -- https://github.com/dart-lang/dartdoc/issues/1196#issuecomment-236949550? Does such a proposal fit within your major overhaul?

jcollins-g commented 7 years ago

@chalin No, but it annoys me as I too often develop under MacOS. There are some other issues here too -- our grind scripts now don't always work properly on case-insensitive filesystems since I tightened the checks. The overhaul ran into this when I was testing it but it doesn't cover the issue. dartdoc just doesn't know it might be overwriting a file when they collide due to case insensitivity.

The major issue I see with your idea is that multiple properties, methods, etc can exist in the same class and differ only in case:

class Foo {
  var foo;
  var Foo;
  var fOo;
 }

Just tagging them with -field won't help us here.

We could disambiguate in a super-ugly, brute-force way by prefixing each capital letter in a filename with an underscore or something. If we wind up special casing it only for colliding identifiers, that's less ugly, but harder for a script to predict. Dartdoc could try to detect whether it is on a case-insensitive filesystem and only fix collisions there, but that's also not great for the same reason.

Probably the first thing to do is have dartdoc emit an error message when this happens so it doesn't silently create bad data, at least.

chalin commented 7 years ago

Probably the first thing to do is have dartdoc emit an error message when this happens so it doesn't silently create bad data, at least.

Agreed.

sigurdm commented 5 years ago

This is popping up in the context of pub: https://github.com/dart-lang/pub/issues/2072.

Maybe we could consider generating fewer files (one per library) this is what godoc does (example: https://golang.org/pkg/bufio/)

kwalrath commented 5 years ago

I would love to have the option of generating fewer files. Personally, I'd like one per class, perhaps with all top-level members in the library page. But for small libraries, one per library seems like a reasonable option.

In large libraries, the sheer number of files generated can cause issues, as we've seen with API docs on webdev.dartlang.org.

devoncarew commented 5 years ago

I would love to have the option of generating fewer files.

Do you mind opening a new issue for that, so we can track it separately from the case sensitivity issue?

kwalrath commented 5 years ago

I would love to have the option of generating fewer files.

Do you mind opening a new issue for that, so we can track it separately from the case sensitivity issue?

1272 covers this. It has details about the issues we ran into in processing the generated files. The implementation has changed, but the general observations are still valid.