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

Add haddock --open flag. #1396 #2015

Merged
merged 1 commit into from
Apr 12, 2016

Conversation

sjakobi
Copy link
Member

@sjakobi sjakobi commented Apr 9, 2016

No description provided.

@sjakobi
Copy link
Member Author

sjakobi commented Apr 9, 2016

The behaviour of the flag is pretty close to this short description by @mgsloan.
The one difference is that when we are in the implicit global project, using the flag with two or more targets opens the haddock index for the global project's snapshot, because the global project doesn't have its own haddock index. As the flag uses the targets for the build command, stack haddock --open in the implicit global project results in the following error message:

~ $ stack haddock --open
Run from outside a project, using implicit global project config
Using resolver: ghc-7.10 from implicit global project's config file: /home/simon/.stack/global-project/stack.yaml
Error parsing targets: The specified targets matched no packages.
Perhaps you need to run 'stack init'?

The flag is not very smart about figuring out the right haddock location when called with a single target. For example, running stack haddock --open base inside the stack project opens /home/simon/.stack/snapshots/x86_64-linux/lts-5.3/7.10.3/doc/base-4.8.2.0/index.html on my computer. I can put in more work here to make sure that (transitive) dependencies of the current project are opened in the project haddocks, if someone thinks that would be desirable.

I was a bit surprised by the behaviour of the command with non-project, non-snapshot targets. For example, when I run stack haddock --open semiring-simple in the stack project, semiring-simple, which isn't included in LTS-5.3, is locally built and its haddocks are locally generated and opened. I'm wondering whether I would prefer an error message in that case, but I may just be overlooking the use case for this behaviour.

@sjakobi sjakobi force-pushed the 1396-haddock-open branch from b63e03d to 3d7b46f Compare April 9, 2016 03:08
@mgsloan
Copy link
Contributor

mgsloan commented Apr 11, 2016

When I run this I get

gvfs-open: /home/mgsloan/fpco/stack/.stack-work/install/x86_64-linux/lts-5.3/7.10.3/doc/"stack-1.0.5"/index.html: error opening location: Error when getting information for file '/home/mgsloan/fpco/stack/.stack-work/install/x86_64-linux/lts-5.3/7.10.3/doc/"stack-1.0.5"/index.html': No such file or directory

I think there is an errant use of show. Also, I don't see why this should need any special code for the global project. It may need to special case compiler-only resolvers, though?

@sjakobi
Copy link
Member Author

sjakobi commented Apr 11, 2016

I think there is an errant use of show.

You're right! I didn't notice because xdg-open strips out the quotes automatically.

Also, I don't see why this should need any special code for the global project.

As I've noted above, the global project doesn't have it's own haddock index file. (This issue is related). So where I would open all/index.html in a local project, I can only offer the snapshot index while in the global project.

Would you prefer a different behaviour for a call like stack haddock --open base containers in the global project?

Aside: Not being able to simply call stack haddock --open in the global project with the expectation to get some useful index is pretty shitty. The reason (as noted in my first comment) is that stack build needs some kind of target. Maybe we should try to add some logic to create a special case for stack haddock --open calls?

It may need to special case compiler-only resolvers, though?

Good point, I hadn't thought about that. extra-deps are already counted as local packages though and therefore opened locally, so it seems we can get away without a special case here.

@sjakobi sjakobi force-pushed the 1396-haddock-open branch from 3d7b46f to ff32ff5 Compare April 11, 2016 22:09
@sjakobi
Copy link
Member Author

sjakobi commented Apr 11, 2016

I think there is an errant use of show.

I've fixed this in combination with a rebase.

@mgsloan
Copy link
Contributor

mgsloan commented Apr 11, 2016

Thanks for fixing!

What do you think of having --open take a package identifier argument (version optional). This argument would implicitly get treated as a target, and we'd open the docs.

Then, we can also have stack haddock --open-index, which could work even when the build step has nothing to build.

One reason I like this is that latter is that later on we can further overload it with opening the docs for a particular module, like stack haddock --open Control.Lens. That said, I already plan to overload the target syntax for stack ghci with something similar. Might consider putting it under stack ghci --module Control.Lens or something, though.

Sorry for the design thrashing, not saying we necessarily need to go this way. But to me it seems like a rather nice way to do it!

@sjakobi
Copy link
Member Author

sjakobi commented Apr 11, 2016

What do you think of having --open take a package identifier argument (version optional). This argument would implicitly get treated as a target, and we'd open the docs.

Then, we can also have stack haddock --open-index, which could work even when the build step has nothing to build.

One reason I like this is that latter is that later on we can further overload it with opening the docs for a particular module, like stack haddock --open Control.Lens. That said, I already plan to overload the target syntax for stack ghci with something similar. Might consider putting it under stack ghci --module Control.Lens or something, though.

I think these are very good ideas. The problem is that I'm not sure when I'll get around to implementing them. I think this PR as it stands is already useful although it certainly has some warts.

Would you consider merging the PR as is and tracking these ideas for improvement in a separate issue?

@mgsloan
Copy link
Contributor

mgsloan commented Apr 12, 2016

Sure. I still don't think there should be logic for the global project, but I'll fix it.

@mgsloan mgsloan merged commit 7696b3a into commercialhaskell:master Apr 12, 2016
@mgsloan
Copy link
Contributor

mgsloan commented Apr 12, 2016

I've fixed the logic to not special case the global project. The global project really has very little that is special about it, it is just a default. Local projects could also have no packages, and run into this same condition.

I've fixed the special casing of global project: 420f86f

I've opened up an issue about the CLI enhancement: #2025

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.

3 participants