Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Display summary and description #504

Closed
wants to merge 1 commit into from

3 participants

☈king Nick Quaranto Erik Michaels-Ober
☈king

As a new gem author, I was pretty puzzled by the contrast between:

This patch takes into account the reality that many authors have been working around the current show logic by putting what should be in the description in the summary: it removes the 'summary' match from the 'description' output.

Thanks!

Erik Michaels-Ober sferik commented on the diff
app/models/version.rb
@@ -135,7 +135,26 @@ def yanked?
end
def info
- [ description, summary, "This rubygem does not have a description or summary." ].detect(&:present?)
Erik Michaels-Ober Owner
sferik added a note

If I'm reading this correctly, the existing code shows the description if both are present, not the summary.

☈king
rking added a note

Yeah, I mis-typed. It's counterintuitive to show a summary iff it's the only thing, then discard it if there's a description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Erik Michaels-Ober sferik commented on the diff
app/views/rubygems/show.html.erb
@@ -8,7 +8,12 @@
<% else %>
<% if @latest_version.indexed %>
<div id="markup">
- <%= simple_markup(@latest_version.info) %>
+ <div id="gem_summary">
+ <%= simple_markup(@latest_version.summary_or_apology) %>
+ </div>
+ <div id="gem_description">
+ <%= simple_markup(@latest_version.description_or_apology) %>
+ </div>
Erik Michaels-Ober Owner
sferik added a note

I'm not sure we need to display both the summary and the description.

☈king
rking added a note

I tried it out with a few existing gems (on my local server), and it seems to be an improvement.

☈king
rking added a note

If this is not the desired behavior, then the "specification" doc should be updated to say something like, "The description takes precedence over the summary in cases where a longer description is needed", then people can start doing like:

gem.description = gem.summary + "\n…"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
☈king

(I re-pushed without the spurious change to this file)

Nick Quaranto
Owner

I think this is obsolete given the redesign. Please rebase/push again if not!

Nick Quaranto qrush closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 3, 2013
  1. ☈king

    Show summary and description separately

    rking authored rking@sharpsaw.org committed
This page is out of date. Refresh to see the latest.
21 app/models/version.rb
View
@@ -135,7 +135,26 @@ def yanked?
end
def info
- [ description, summary, "This rubygem does not have a description or summary." ].detect(&:present?)
Erik Michaels-Ober Owner
sferik added a note

If I'm reading this correctly, the existing code shows the description if both are present, not the summary.

☈king
rking added a note

Yeah, I mis-typed. It's counterintuitive to show a summary iff it's the only thing, then discard it if there's a description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ text_or_apology_for :summary, :description
+ end
+
+ def summary_or_apology
+ text_or_apology_for :summary
+ end
+
+ def description_or_apology
+ desc = text_or_apology_for :description
+ if summary
+ desc.sub summary, ''
+ else
+ desc
+ end
+ end
+
+ def text_or_apology_for *attrs
+ apology = "This rubygem does not have a #{attrs.map(&:to_s).join ' or '}."
+ candidates = attrs.map { |e| send e } + [ apology ]
+ candidates.detect(&:present?)
end
def update_attributes_from_gem_specification!(spec)
7 app/views/rubygems/show.html.erb
View
@@ -8,7 +8,12 @@
<% else %>
<% if @latest_version.indexed %>
<div id="markup">
- <%= simple_markup(@latest_version.info) %>
+ <div id="gem_summary">
+ <%= simple_markup(@latest_version.summary_or_apology) %>
+ </div>
+ <div id="gem_description">
+ <%= simple_markup(@latest_version.description_or_apology) %>
+ </div>
Erik Michaels-Ober Owner
sferik added a note

I'm not sure we need to display both the summary and the description.

☈king
rking added a note

I tried it out with a few existing gems (on my local server), and it seems to be an improvement.

☈king
rking added a note

If this is not the desired behavior, then the "specification" doc should be updated to say something like, "The description takes precedence over the summary in cases where a longer description is needed", then people can start doing like:

gem.description = gem.summary + "\n…"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
</div>
<div class="border">
3  public/stylesheets/screen.css
View
@@ -534,6 +534,9 @@ table {
list-style: inside disc;
}
+#gem_summary {
+ border-bottom: 1px solid #DAD0BD;
+}
.main .info p {
color: #5e543e;
13 test/unit/version_test.rb
View
@@ -244,24 +244,35 @@ class VersionTest < ActiveSupport::TestCase
should "have description for info" do
@version.description = @info
assert_equal @info, @version.info
+ assert_equal @info, @version.description_or_apology
end
should "have summary for info if description does not exist" do
@version.description = nil
@version.summary = @info
assert_equal @info, @version.info
+ assert_equal @info, @version.summary_or_apology
end
should "have summary for info if description is blank" do
@version.description = ""
@version.summary = @info
assert_equal @info, @version.info
+ assert_equal @info, @version.summary_or_apology
+ end
+
+ should "filter out the bad habit of putting the summary in the desc" do
+ @version.description = @info + 'etc.'
+ @version.summary = @info
+ assert_equal 'etc.', @version.description_or_apology
end
should "have some text for info if neither summary or description exist" do
@version.description = nil
@version.summary = nil
- assert_equal "This rubygem does not have a description or summary.", @version.info
+ assert_equal "This rubygem does not have a summary or description.", @version.info
+ assert_equal 'This rubygem does not have a summary.', @version.summary_or_apology
+ assert_equal 'This rubygem does not have a description.', @version.description_or_apology
end
context "when yanked" do
Something went wrong with that request. Please try again.