Include readme #356
Include readme #356
Conversation
…og in `PackageRender`
…the contents of a toplevel file matching a given filter
…ageContents`. This eliminates unnecessarily threading `findToplevelFile` through `PackageContents`. The other solution would be to have `RecentPackages` depend on `TarIndexCache` directly.
The pattern files are tested against is in `Packages.Readme`. Both candidates and normal packages have the contents of both their Changelog and Readme incorporated into `PackageRender`.
…e-server into include-readme Conflicts: Distribution/Server/Features/RecentPackages.hs Distribution/Server/Packages/ChangeLog.hs Distribution/Server/Packages/Render.hs Distribution/Server/Pages/Package.hs Distribution/Server/Util/ServeTarball.hs hackage-server.cabal
Personally, I think it makes more sense to leave off the description when the README contents are available, but this was the only comment against haskell#333 being merged, so I'm adding this commit to be on the safe side. Feel free to revert/rebase to exclude, I'm fine either way.
|
👍 |
|
Great, thanks for testing it. |
|
I'm eager to see this in action :) |
|
Ok, so I tried it out and poked about at various examples and unfortunately it's not going to be quite so simple. There's lots of packages with readmes that either duplicate or are just worse than their descriptions. So for the moment I've got it only linking to the readme, though rendering the markdown. My suggestion for doing better is this:
Anyway, all the code is merged, and the presentation is already hopefully an improvement. We can work on the visuals as a follow-up. Patches welcome. |
|
Well... that sounds like I just wasted my time, as did everyone else who worked on this PR. Can you let me know when this is deployed on the live site so I can see how useless this feature turned out to be? |
|
Oh I don't think it's a waste of time at all, it just needs more tweaking before we can use it for all packages. For some packages it of course looked great (those that have no description and a good readme, like many of your packages) and for other packages it looked terrible (those with both a description and a long or similar readme). It's deployed now. |
|
To be clear, I'm not saying what's deployed now is how things must remain. On the contrary. I wanted to get it merged, but looking at the results as it was it was a significant regression. So I think this half-way house is better than not applying it and asking people to revise and resubmit (which would be really momentum-sapping). This gives us a good position to make the extra tweaks so that we can use the readme inline in the places where it's appropriate and so that it looks good. |
|
Fwiw, we already had the pushing-down issue w/ http://hackage.haskell.org/package/lens which has quite a large description-field... I think both, the show first N lines suggestion as well as the "when the description is empty or ultra-short, use the readme inline, otherwise just link to it." heuristic would be useful. ...and I'm still wondering how we can fix I assume we can now link to READMEs from the |
|
@snoyberg btw, does the mardown renderer not support html tags in markdown, see e.g. the Examples section in http://hackage.haskell.org/package/lens-4.11/readme |
|
@snoyberg btw, here's an example of a rather inappropriate README for Hackage: http://hackage.haskell.org/package/filediff-1.0.0.2/readme The problem I see is that some READMEs are more tailored towards use on GitHub, and contain information not relevant for display on Hackage. Maybe we should communicate the intent by having a Here's a different kind of README, http://hackage.haskell.org/package/docopt-0.7.0.2/readme So we seem to have various kinds of README, with quite different intents... |
|
I didn't write the code to call the Markdown renderer, nor was I part of the decision to use that one. Switching to something better is trivial. But I've already wasted more time on this PR and related issues than I care to. It's pretty obvious that decisions are being made after work is invested by others, without letting others be involved in that decision making process. "momentum-sapping" is certainly the right word here; letting the PR bitrot in the first place, or the lack of clear guidance to the people working on it in the first place, being prime examples. I'll go figure out others ways to be productive and improve Haskell documentation that don't involve this PR. |
|
@snoyberg The main issue is that many packages out there have either a comprehensive description or a comprehensive e.g. Adding the |
|
To be clear, there might be a misunderstanding of intent here:
I think the problem of the length of the README can be avoided by putting it at the end of the page. The fact that some packages have massive metadata that you have to scroll down is an orthogonal problem with the package page itself that needs solving (e.g. tabs or w/e) elsewhen. |
|
@ekmett I'm not sure where things were moving along productively previously. I was involved from the beginning. This has been lethargic from the outset. Regarding your comment about having README + description being too much and redundant information: I agree completely. I think it's a mistake. But the previous PR seemed to have stalled/bitrotted because of a single comment from Herbert about that. So I decided to try and overcome it by adding the description in this PR. I even commented in both the commit itself and this PR that I thought it was a problem.
I want to make sure I'm understanding your intent here correctly: we should make sure that the only way to get content onto a package's page is via a very difficult-to-use format (cabal+Haddock) and must be duplicated with the README content we all (yourself included) are using already, because there's extra information there that someone may not care to see on the Hackage page? |
|
Re And fwiw, re "without letting others be involved in that decision making process", @dcoutts did in fact reach out on While adding READMEs via @chrisdone re
I still think that the The problem w/ the description-only-in-README I see is that it's not part of the index-tarball anymore, hence you can't access it anymore via tooling such as |
|
And just because this irks me so much: why are we having this conversation now? The issues about this have been open since September. Anyone could have commented about their concerns around implementing this back then. Instead, we're now- after three people spent significant time implementing the feature- having discussions that could have trivially been had before the work started. This is inconsiderate at best. The result we have now- a hard-to-find link to a README- is something I have been able to implement myself for years by just putting a URL in my description field. There was no need to waste my time implementing it if we were just going to end up in this situation. |
|
@hvr The desire to have offline metadata about packages is one I've heard only two people express. Nonetheless, I already solved that problem for you: https://github.com/commercialhaskell/all-cabal-metadata. I'd send a PR to cabal about that, but I don't exactly have a great track record of writing PRs that actually get included in a useful way. |
|
@snoyberg The only reason I got involved in this discussion at all was because I happened to look at the #hackage channel when @dcoutts was discussing things with @cartazio about how it looked in practice. I think we all like the general idea, but there is a great deal of concern about precisely the right way to go about this. Automatically picking between the description and the Putting up a link to the It leaves open options such as
All of those options kind of suck, but you have to admit that the proposal as it stands has rough edges of its own. Why are these coming up now rather than back when this was first proposed? It isn't an attempt to curb stomp this idea or block momentum. It is that folks finally had a concrete proposal to poke holes in. They could see what it looked like and some of the results gave people pause. |
|
FWIW, it was a conscious decision to disallow raw HTML, it can be changed by a single flag. What you describe as "Hackage's inability to display documentation correctly" is actually a result of the incredible fragmentation of markdown specifications. Pandoc is another example of a package which looks terrible with these changes. The README is written in it's own markdown dialect. |
@snoyberg Look, this was certainly not my intention, rather the opposite. Yes this patch had been hanging around for some time (partly because it was quite large and partly because we didn't have good answers to this question of when to use the README). Myself and others greatly appreciate that you and @mpickering rescued @conklech's patch from bitrot. I spent some hours reviewing and then testing the patch, and then looking at this policy question of when to show the README. My priority was to get the patch in so we didn't suffer further bitrot and disheartenment for the patch authors. The detail of when the README is shown did not need o hold up getting the bulk of the code merged. So yes, I could have asked you and @mpickering and people to go back and think about this before merging. But I thought it was better to ask that after merging. That's what I did. I did also ask other people for an immediate opinion, that wouldn't delay getting the code merged. That's why @ekmett and @cartazio gave me their 2c, because I asked for immediate feedback on IRC.
That's what we're doing right now, after having merged the bulk of the code, leaving just the policy question to be decided. I'm sorry that I apparently gave the opposite impression. As for why now, rather than right at the beginning. Honestly, it wasn't until I looked at the result that it was obvious that using both the description and the readme together gave bad results in many cases. Could this have been predicted from the start? I guess. But seriously it's not that huge an issue. Deciding when to show the README is not a massive change to the code here, and not a waste of everyone's work. I've made a suggestion above about a policy of when to show the README inline, and that would not be hard to do. The issue of long READMEs/descriptions was in fact identified much earlier (when @mpickering picked up the patch). |
|
@chrisdone Thanks for the clarifications. Yes, I concur with your identification of Problem 1, 2 and the solution, and I have no problem with the ultimate goal of letting people use a README in place of the description from the .cabal file.
So I don't think there's a misunderstanding of the goals here. The intermediate problem is that while many packages do have good READMEs (and some poor descriptions), many packages do not have good READMEs. We can encourage a move to using README, but if we were to flip the switch on all old packages the results are pretty horrible (I've seen them). Hence my suggestion that we use the README when the description is empty/ultra-short. That will work now for many packages, and makes it easy to opt-in, even for released packages (by editing the .cabal file). To further encourage the switch we could have Cabal add a "description-file: README" field, and tell authors they should either have a
As I've tried to make clear, the current state is just a half way house, to get the code merged, pending working out the detail of when to show the README inline. Perhaps it'd have been less controversial if I'd merged the code but simply disabled the new feature initially and not added the README link. I thought it was helpful but apparently it has confused matters about what the ultimate intention is. Sorry about that. And yes, I agree with you about the length issue. A page layout redesign would make it a lot easier to deal with long descriptions and READMEs. The current layout is predicated on a modest description. A layout that someone mocked up before http://code.haskell.org/~duncan/hackage-server-new-chrome/package.html has the properties at the side which leaves a lot more room for the description/readme. I think something like this would be nice. Patches welcome :-) |
|
@wiz We're spoilt for choice! :-) We've got pandoc, cheapskate and cmark. No problems here with switching to a different one if it gets us generally better results. That part of the code is quite well isolated, so feel free to send a patch. |
Oh, this is nice. I would like to see a modern CSS framework (BS3, materialize, etc...) being utilized. Is there a repo i can fork?
I think we should aim for |
|
@wiz I'd love someone to pick up this redesign work. There's no repo, it was done ad-hoc by someone. I just preserved it. So look at the files in http://code.haskell.org/~duncan/hackage-server-new-chrome/
Vote noted. I don't have a particular opinion on this, whatever works. Hopefully we can decide quickly and just do it. Shouldn't be hard to change. In fact perhaps while we've got the README not yet being on the front page we should take the opportunity to try the other markdown impls and then pick the one that works best for the files we've got. Pull reqs accepted! |
|
My 2 cents on pandoc vs. cheapskate vs. cmark (since I'm the author of all of them):
Note that if you use pandoc or cmark, you should combine it with xss-sanitize. You might also consider the possibility of rendering client-side using jgm/commonmark.js, which should render exactly the same way |
|
@jgm thanks very much for the detailed advice. |
|
I discussed this feature at length with Duncan just now, and he asked me to write in my thoughts. I have exactly one thing I care about, which is getting the README content onto the Hackage package pages, so that I can start encouraging package authors to easily improve the documentation for their packages. I'm OK if the content appears below the metadata. I'm OK if there's an expander added. In fact, I haven't seen a single suggestion here that I'm opposed to. I just want something to happen. |
|
So here's my current suggestion:
I think if we have both the description and reame, we can encourage people to have short descriptions, and put all the longer detail into the readme/extended docs. The original idea of the description was never to be a tutorial or whatever but to be a simple paragraph summary (with the synopsis being a very terse one-line summary). Since we've always only had the description on hackage and not had a good place for other docs (haddock api docs arn't great for intro tutorials and other misc stuff), then this has encouraged people to stuff more and more things into the description, to the point where they suffer from the poor markup and we have duplication between description and reame (and consequent annoyance from people about that repetition, and the poor markup etc). On the other hand people have pointed out that the description does serve a purpose, and has a slightly different audience and context. So hopefully if we do display both we can get the descriptions back down to a managable size (e.g. one paragraph) and give lots of space and nicer markup for more and better intro & misc docs. If people think this is a good approach (short description, everything else in readme) then we can modify Cabal's Q/A checks to encourage people in that direction, e.g. complaining about long descriptions and pointing people towards using markdown readmes. Also this approach has the advantage that it's easy to implement, which is important to get it done swiftly. Thoughts? |
|
Agreed with all that. I like this approach. I'd suggest after the module list and bite the bullet on packages like lens where the module list is super long. |
|
Another "like Github" thing we could do: add a "Read more" link right after the synopsis or description that would take you down to the README content. |
|
Here is what the lens package looks like with the README taken from Github and dumped at the end of the page before the footer (and the description cut down to a paragraph):
|
@snoyberg I like that. I was also a little worried that if we put it at the end that people simply will not notice. So I could replace the "readme" link to the out-of-line reamde with a "read more" link to that anchor within the page. @chrisdone ok, thanks for the vote. Yeah, lens is crazy, but once we've got the readme in, I'm sure Edward can be persuaded to prune the description a bit (aeson is another offender in this area). |
|
@dcoutts By including the screenshot I meant more in a "this is okay" way. It's not that bad. So I think this'll work out nicely. |
|
@chrisdone yes, I misread at first. I get what you mean. And the good thing is that we can edit descriptions in the .cabal files to prune them where appropriate. |
|
I'm willing to take a hacksaw to the descriptions in my |
|
Does the ability to edit package meta-data in a revision extend to editing the descriptions or just the dependency stuff? If not could it be extended there? |
|
@ekmett I believe I set it up so that editing the descriptions and (other similar metadata) is allowed. The principle being we're fairly liberal about changes to data that doesn't affect the build, and very cautious on stuff that does affect the build (only deps I think, nothing else iirc). |
That means two fronts:
@dcoutts What's the status on either of those fronts? Need any help on either? |
|
@mboes the change to put the README inline is now done (pending deployment). Changes to cabal to encourage good synopsis & description is yet to do. A pull req on that would be appreciated. I think the rough consensus on the latter is this:
And in particular, changelogs should not go in the description, they should go in a changelog(.md). A message in cabal check could perhaps also mention that markdown is encouraged for changelogs and readmes. The current hackage code will render markdown files as markdown, and all other extensions as plain text. Rationale: synopsis is useful in many places for a very brief idea of what a package is about. Description is again useful for users to quickly get an idea about whether the package is relevant to them. The description gets downloaded to all clients so can be used in list/search features, which is harder for READMEs that live inside package tarballs. |
All I've done is updated PR #333 to the latest master, and added an extra commit to keep both the description and the README content.