[search] Update list of features skipped from search

Signed-off-by: Konstantin Pastbin <konstantin.pastbin@gmail.com>

indexer/ftypes_matcher.cpp

@@ -353,8 +353,11 @@ IsStreetOrSquareChecker::IsStreetOrSquareChecker()
     m_types.push_back(t);
 }
 
+// Used to determine for which features to display address in PP and in search results.
+// If such a feature has a housenumber and a name then its enriched with a postcode (at the generation stage).
 IsAddressObjectChecker::IsAddressObjectChecker() : BaseChecker(1 /* level */)
 {
+  /// @todo(pastk): some objects in TwoLevelPOIChecker can have addresses also.
   m_types = OneLevelPOIChecker().GetTypes();
 
   Classificator const & c = classif();
@@ -362,6 +365,8 @@ IsAddressObjectChecker::IsAddressObjectChecker() : BaseChecker(1 /* level */)
     m_types.push_back(c.GetTypeByPath({p}));
 }
 
+// Used to insert exact address (street and house number) instead of
+// an empty name in search results (see ranker.cpp)
 IsAddressChecker::IsAddressChecker() : BaseChecker(1 /* level */)
 {
   Classificator const & c = classif();
@@ -431,6 +436,7 @@ IsPisteChecker::IsPisteChecker() : BaseChecker(1 /* level */)
 }
 
 
+// Used in IsPoiChecker and in IsAddressObjectChecker.
 OneLevelPOIChecker::OneLevelPOIChecker() : ftypes::BaseChecker(1 /* level */)
 {
   Classificator const & c = classif();
@@ -440,6 +446,7 @@ OneLevelPOIChecker::OneLevelPOIChecker() : ftypes::BaseChecker(1 /* level */)
     m_types.push_back(c.GetTypeByPath({path}));
 }
 
+// Used in IsPoiChecker and also in TypesSkipper to keep types in the search index.
 TwoLevelPOIChecker::TwoLevelPOIChecker() : ftypes::BaseChecker(2 /* level */)
 {
   Classificator const & c = classif();

indexer/ftypes_matcher.hpp

@@ -291,6 +291,10 @@ public:
   TwoLevelPOIChecker();
 };
 
+// Used in:
+// - search ranking (POIs are ranked somewhat higher than "unclassified" features, see search/model.hpp)
+// - add type's translation into complex PP subtitle (see GetLocalizedAllTypes())
+// - building lists of popular places (generator/popular_places_section_builder.cpp)
 class IsPoiChecker
 {
 public:

search/search_integration_tests/smoke_test.cpp

@@ -185,6 +185,9 @@ UNIT_CLASS_TEST(SmokeTest, TypesSkipperTest)
 
 UNIT_CLASS_TEST(SmokeTest, CategoriesTest)
 {
+  // Checks all types in categories.txt for their searchability,
+  // which also depends on point drawability and presence of a name.
+
   auto const & cl = classif();
 
   /// @todo Should rewrite test
@@ -246,13 +249,17 @@ UNIT_CLASS_TEST(SmokeTest, CategoriesTest)
       {"highway", "motorway_junction"},
       {"landuse"},
       {"man_made", "chimney"},
+      {"man_made", "flagpole"},
+      {"man_made", "mast"},
+      {"man_made", "tower"},
+      {"man_made", "water_tower"},
       {"natural"},
       {"office"},
       {"place"},
       {"waterway"},
 
       /// @todo Controversial here.
-      /// Don't have point drawing rules except text -> type will be removed for Feature with empty name.
+      /// Don't have point drawing rules (a label only), hence type will be removed for an empty name Feature.
       {"building", "train_station"},
       {"leisure", "track"},
       {"natural", "beach"},

search/types_skipper.cpp

@@ -15,16 +15,24 @@ TypesSkipper::TypesSkipper()
 {
   Classificator const & c = classif();
 
+  /// @todo(pastk): why "office-*" are skipped?
+  /// also could be unnamed (yet) place-hamlet/isolated_dwelling?
+  /// and natural-water-pond/lake?
+  // POIs like natural-spring, waterway-waterfall, highway-bus_stop are saved from skipping by the TwoLevelPOIChecker().
   StringIL const arrSkipEmptyName1[] = {
     {"area:highway"}, {"building"}, {"highway"}, {"landuse"}, {"natural"}, {"office"}, {"place"}, {"waterway"},
   };
   for (auto const & e : arrSkipEmptyName1)
     m_skipIfEmptyName[0].push_back(c.GetTypeByPath(e));
 
-  StringIL const arrSkipEmptyNameExact[] = {
+  StringIL const arrSkipEmptyName2[] = {
     {"man_made", "chimney"},
+    {"man_made", "flagpole"},
+    {"man_made", "mast"},
+    {"man_made", "tower"},
+    {"man_made", "water_tower"},
   };
-  for (auto const & e : arrSkipEmptyNameExact)
+  for (auto const & e : arrSkipEmptyName2)
     m_skipIfEmptyName[1].push_back(c.GetTypeByPath(e));
 
   m_skipAlways[0].push_back(c.GetTypeByPath({"isoline"}));
@@ -44,6 +52,7 @@ void TypesSkipper::SkipEmptyNameTypes(feature::TypesHolder & types) const
     if (m_isPoi(type))
       return false;
 
+    ftype::TruncValue(type, 2);
     if (HasType(m_skipIfEmptyName[1], type))
       return true;
 

search/types_skipper.hpp

@@ -8,18 +8,20 @@
 
 namespace search
 {
-// Skips some feature's types when feature name is empty.
+// Helper functions to determine if a feature should be indexed for search.
 class TypesSkipper
 {
 public:
   TypesSkipper();
 
+  // Removes types which shouldn't be searchable in case there is no feature name.
   void SkipEmptyNameTypes(feature::TypesHolder & types) const;
 
   // Always skip feature for search index even it has name and other useful types.
   // Useful for mixed features, sponsored objects.
   bool SkipAlways(feature::TypesHolder const & types) const;
 
+  // Skip "entrance" only features if they have no name or a number ref only.
   bool SkipSpecialNames(feature::TypesHolder const & types, std::string_view defName) const;
 
 private: