Skip to content

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

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

Open
chalin opened this issue Jul 12, 2016 · 15 comments
Open

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

chalin opened this issue Jul 12, 2016 · 15 comments
Labels
P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@chalin
Copy link
Contributor

chalin commented Jul 12, 2016

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:

  • angular2.common/NgIf/NgIf.html
  • angular2.common/NgIf/ngIf.html

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
Copy link
Collaborator

keertip commented Jul 14, 2016

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

@chalin
Copy link
Contributor Author

chalin commented Jul 14, 2016

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

@keertip keertip added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Jul 21, 2016
@keertip keertip added this to the hackathon milestone Jul 21, 2016
@devoncarew
Copy link
Member

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
Copy link
Contributor Author

chalin commented Aug 1, 2016

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
Copy link
Contributor

kwalrath commented Aug 2, 2016

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
Copy link
Contributor Author

chalin commented Aug 2, 2016

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
Copy link
Contributor

kwalrath commented Aug 5, 2016

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

@devoncarew devoncarew modified the milestone: hackathon Nov 22, 2016
@jcollins-g
Copy link
Contributor

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

@chalin
Copy link
Contributor Author

chalin commented Apr 11, 2017

Yes, the issue arose because @kwalrath and I mainly build under MacOS. What do you think of the proposal given a few comments above -- #1196 (comment)? Does such a proposal fit within your major overhaul?

@jcollins-g jcollins-g changed the title Wrong page content for class when it has member w/ same name but different case/capitalization dartdoc can generate broken docs on case-insensitive filesystems Apr 11, 2017
@jcollins-g
Copy link
Contributor

@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.

@jcollins-g jcollins-g added the P2 A bug or feature request we're likely to work on label Apr 11, 2017
@chalin
Copy link
Contributor Author

chalin commented Apr 11, 2017

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
Copy link

sigurdm commented Mar 8, 2019

This is popping up in the context of pub: dart-lang/pub#2072.

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

@kwalrath
Copy link
Contributor

kwalrath commented Mar 8, 2019

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
Copy link
Member

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
Copy link
Contributor

kwalrath commented Mar 8, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants