From 918f32c554cab7325271520db70d3d6257c5f61a Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 30 Apr 2022 01:44:42 -0500 Subject: [PATCH] Fix #4461: Improve posts/index page titles. --- app/helpers/application_helper.rb | 8 +- app/logical/post_query.rb | 2 +- app/logical/post_query/ast.rb | 26 ++++++- app/logical/post_query_builder.rb | 94 ------------------------ app/logical/post_sets/post.rb | 21 +++++- app/views/posts/index.html.erb | 7 +- test/functional/posts_controller_test.rb | 4 +- test/unit/post_query_builder_test.rb | 18 ----- 8 files changed, 54 insertions(+), 126 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d31985525..a0195440d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -123,13 +123,13 @@ module ApplicationHelper end end - def humanized_number(number) + def humanized_number(number, million: "M", thousand: "k") if number >= 1_000_000 - format("%.1fM", number / 1_000_000.0) + format("%.1f#{million}", number / 1_000_000.0) elsif number >= 10_000 - "#{number / 1_000}k" + "#{number / 1_000}#{thousand}" elsif number >= 1_000 - format("%.1fk", number / 1_000.0) + format("%.1f#{thousand}", number / 1_000.0) else number.to_s end diff --git a/app/logical/post_query.rb b/app/logical/post_query.rb index c3335f367..fe7972aec 100644 --- a/app/logical/post_query.rb +++ b/app/logical/post_query.rb @@ -18,7 +18,7 @@ class PostQuery attr_reader :current_user private attr_reader :tag_limit, :safe_mode, :hide_deleted_posts, :builder - delegate :tag?, :metatag?, :wildcard?, :metatags, :wildcards, :tag_names, :to_infix, to: :ast + delegate :tag?, :metatag?, :wildcard?, :metatags, :wildcards, :tag_names, :to_infix, :to_pretty_string, to: :ast alias_method :safe_mode?, :safe_mode alias_method :hide_deleted_posts?, :hide_deleted_posts alias_method :to_s, :to_infix diff --git a/app/logical/post_query/ast.rb b/app/logical/post_query/ast.rb index ef9f1a365..6ba540562 100644 --- a/app/logical/post_query/ast.rb +++ b/app/logical/post_query/ast.rb @@ -286,6 +286,30 @@ class PostQuery end end + # Display the search in "pretty" form, with capitalized tags. + def to_pretty_string + case self + in [:all] + "" + in [:none] + "none" + in [:wildcard, name] + name + in [:tag, name] + name.tr("_", " ").gsub(/\b([a-z])/, &:capitalize) + in [:metatag, name, value, quoted] + "#{name}:#{quoted_value}" + in :not, child + child.term? ? "-#{child.to_pretty_string}" : "-(#{child.to_pretty_string})" + in :opt, child + child.term? ? "~#{child.to_pretty_string}" : "~(#{child.to_pretty_string})" + in :and, *children + children.map { _1.children.many? ? "(#{_1.to_pretty_string})" : _1.to_pretty_string }.to_sentence + in :or, *children + children.map { _1.children.many? ? "(#{_1.to_pretty_string})" : _1.to_pretty_string }.to_sentence(two_words_connector: " or ", last_word_connector: ", or ") + end + end + # Convert the AST to a series of nested arrays. def to_tree if term? @@ -445,6 +469,6 @@ class PostQuery end end - memoize :to_cnf, :simplify, :simplify_once, :rewrite_opts, :trim, :trim_once, :sort, :inquirer, :deconstruct, :inspect, :to_sexp, :to_infix, :to_tree, :nodes, :tags, :metatags, :tag_names, :parents + memoize :to_cnf, :simplify, :simplify_once, :rewrite_opts, :trim, :trim_once, :sort, :inquirer, :deconstruct, :inspect, :to_sexp, :to_infix, :to_pretty_string, :to_tree, :nodes, :tags, :metatags, :tag_names, :parents end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 287da058e..ac739a262 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -493,87 +493,6 @@ class PostQueryBuilder end concerning :ParseMethods do - # Parse the search into a list of search terms. A search term is a tag or a metatag. - # @return [Array] a list of terms - def scan_query - terms = [] - query = query_string.to_s.gsub(/[[:space:]]/, " ") - scanner = StringScanner.new(query) - - until scanner.eos? - scanner.skip(/ +/) - - if scanner.scan(/(-)?(#{METATAGS.join("|")}):/io) - operator = scanner.captures.first - metatag = scanner.captures.second.downcase - value, quoted = scan_string(scanner) - - if metatag.in?(COUNT_METATAG_SYNONYMS) - metatag = metatag.singularize + "_count" - elsif metatag == "order" - attribute, direction, _tail = value.to_s.downcase.partition(/_(asc|desc)\z/i) - if attribute.in?(COUNT_METATAG_SYNONYMS) - value = attribute.singularize + "_count" + direction - end - end - - terms << OpenStruct.new(type: :metatag, name: metatag, value: value, negated: (operator == "-"), quoted: quoted) - elsif scanner.scan(/([-~])?([^ ]+)/) - operator = scanner.captures.first - tag = scanner.captures.second - terms << OpenStruct.new(type: :tag, name: tag.downcase, negated: (operator == "-"), optional: (operator == "~"), wildcard: tag.include?("*")) - elsif scanner.scan(/[^ ]+/) - terms << OpenStruct.new(type: :tag, name: scanner.matched.downcase) - end - end - - terms - end - - # Parse a single-quoted, double-quoted, or unquoted string. Used for parsing metatag values. - # @param scanner [StringScanner] the current parser state - # @return [Array<(String, Boolean)>] the string and whether it was quoted - def scan_string(scanner) - if scanner.scan(/"((?:\\"|[^"])*)"/) - value = scanner.captures.first.gsub(/\\(.)/) { $1 } - quoted = true - elsif scanner.scan(/'((?:\\'|[^'])*)'/) - value = scanner.captures.first.gsub(/\\(.)/) { $1 } - quoted = true - else - value = scanner.scan(/(\\ |[^ ])*/) - value = value.gsub(/\\ /) { " " } - quoted = false - end - - [value, quoted] - end - - # Split the search query into a list of strings, one per search term. - # Roughly the same as splitting on spaces, but accounts for quoted strings. - # @return [Array] the list of terms - def split_query - terms.map do |term| - type, name, value = term.type, term.name, term.value - - str = "" - str += "-" if term.negated - str += "~" if term.optional - - if type == :tag - str += name - elsif type == :metatag && (term.quoted || value.include?(" ")) - value = value.gsub(/\\/) { '\\\\' } - value = value.gsub(/"/) { '\\"' } - str += "#{name}:\"#{value}\"" - elsif type == :metatag - str += "#{name}:#{value}" - end - - str - end - end - class_methods do # Parse a simple string value into a Ruby type. # @param string [String] the value to parse @@ -700,17 +619,4 @@ class PostQueryBuilder end end end - - concerning :UtilityMethods do - def to_s - split_query.join(" ") - end - - # The list of search terms. This includes regular tags and metatags. - def terms - @terms ||= scan_query - end - end - - memoize :split_query end diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index 9e136848d..8fcdc2d0d 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -6,6 +6,8 @@ # @see PostsController#index module PostSets class Post + extend Memoist + MAX_PER_PAGE = 200 MAX_SIDEBAR_TAGS = 25 MAX_WILDCARD_TAGS = PostQueryBuilder::MAX_WILDCARD_TAGS @@ -25,8 +27,21 @@ module PostSets @show_votes = show_votes end - def humanized_tag_string - query.split_query.map { |tag| tag.tr("_", " ").titleize }.to_sentence + # The title of the page for the tag. + def page_title + post_query.to_pretty_string + end + + # The description of the page for the <meta name="description"> tag. + def meta_description + if post_query.is_simple_tag? + humanized_count = ApplicationController.helpers.humanized_number(post_count, million: " million", thousand: " thousand") + humanized_count = "over #{humanized_count}" if post_count >= 1_000 + + "See #{humanized_count} #{page_title} images on #{Danbooru.config.app_name}. #{DText.excerpt(wiki_page&.body)}" + else + ApplicationController.helpers.site_description + end end def has_blank_wiki? @@ -170,5 +185,7 @@ module PostSets searches.take(MAX_SIDEBAR_TAGS) end end + + memoize :page_title end end diff --git a/app/views/posts/index.html.erb b/app/views/posts/index.html.erb index a10c992c9..1a9e1684c 100644 --- a/app/views/posts/index.html.erb +++ b/app/views/posts/index.html.erb @@ -228,10 +228,9 @@ <% atom_feed_tag "Posts", posts_url(format: :atom) %> <% else %> - <% page_title("#{@post_set.humanized_tag_string} Art") %> - <% meta_description("See over #{number_with_delimiter(@post_set.post_count)} #{@post_set.humanized_tag_string} images on #{Danbooru.config.app_name}. #{DText.excerpt(@post_set.wiki_page&.body)}") %> - - <% atom_feed_tag "Posts: #{@post_set.tag_string}", posts_url(tags: @post_set.tag_string, format: :atom) %> + <% page_title(@post_set.page_title) %> + <% meta_description @post_set.meta_description %> + <% atom_feed_tag "Posts: #{@post_set.page_title}", posts_url(tags: @post_set.tag_string, format: :atom) %> <% end %> <% if params[:tags].blank? && @post_set.current_page == 1 %> diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 17debc62a..c2d2444cc 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -467,8 +467,8 @@ class PostsControllerTest < ActionDispatch::IntegrationTest context "in safe mode" do should "not include the rating:s tag in the page title" do - get posts_path(tags: "1girl", safe_mode: true) - assert_select "title", text: "1girl Art | Safebooru" + get posts_path(tags: "fate/grand_order", safe_mode: true) + assert_select "title", text: "Fate/Grand Order | Safebooru" end end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index ef78ac6a6..e83063bbc 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -13,13 +13,6 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_equal(count, PostQuery.normalize(query, **query_options).with_implicit_metatags.fast_count(**fast_count_options)) end - def assert_parse_equals(expected, query) - assert_equal(expected, PostQueryBuilder.new(query).split_query) - - # parsing, serializing, then parsing again should produce the same result. - assert_equal(PostQuery.new(query).to_s, PostQuery.new(PostQuery.new(query).to_s).to_s) - end - setup do CurrentUser.user = create(:user) CurrentUser.ip_addr = "127.0.0.1" @@ -1387,17 +1380,6 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end end - context "Parsing:" do - should "split a query" do - assert_equal(%w(aaa bbb), PostQueryBuilder.new("aaa bbb").split_query) - end - - should "not strip out valid characters when scanning" do - assert_equal(%w(aaa bbb), PostQueryBuilder.new("aaa bbb").split_query) - assert_equal(%w(favgroup:yondemasu_yo,_azazel-san. pool:ichigo_100%), PostQueryBuilder.new("favgroup:yondemasu_yo,_azazel-san. pool:ichigo_100%").split_query) - end - end - context "#fast_count" do setup do create(:tag, name: "grey_skirt", post_count: 100)