GNOME.org
 

[glom] TranslatableItems: Make less methods virtual.

commit 37c8bf386ca8732098a49396664778ae352260b6
Author: Murray Cumming <murrayc murrayc com>
Date:   Mon Jan 16 11:42:18 2012 +0100

    TranslatableItems: Make less methods virtual.
    
    * glom/libglom/data_structure/translatable_item.[h|cc]:
    get_title_translation(), get_title_or_name_original():
    Make these non-virtual, because nothing needs to override them.
    * glom/libglom/data_structure/layout/layoutitem_field.[h|cc]:
      Remove useless get_title_original(), get_title_translation(), and
      get_title_or_name_original() overrides.
      Remove useless and unused private get_title_no_custom_original() method.
    * glom/libglom/document/document.[h|cc]: Add a
      fill_translatable_layout_items(LayoutItem_Field) method, and use it in
      fill_translatable_layout_items(LayoutGroup), instead of repeating code.
      Avoid calling get_title_original() and get_title_translation() on
      LayoutItem_Field because it does not have its own title.
    * glom/libglom/example_document_load.cc:
    * tests/test_document_load_translations.cc: Adapted.

 ChangeLog                                          |   19 +++++++
 .../data_structure/layout/layoutitem_field.cc      |   51 --------------------
 .../data_structure/layout/layoutitem_field.h       |   17 ++-----
 .../data_structure/layout/layoutitem_portal.h      |    7 ++-
 .../layout/report_parts/layoutitem_fieldsummary.h  |    2 -
 glom/libglom/data_structure/translatable_item.cc   |    9 ----
 glom/libglom/data_structure/translatable_item.h    |    5 +-
 glom/libglom/document/document.cc                  |   51 ++++++++++++-------
 glom/libglom/document/document.h                   |    1 +
 glom/libglom/example_document_load.cc              |    6 +-
 tests/test_document_load_translations.cc           |   45 +++++++++++------
 11 files changed, 96 insertions(+), 117 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index a6f74ef..49c8a85 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2012-01-16  Murray Cumming  <murrayc murrayc com>
 
+	TranslatableItems: Make less methods virtual.
+
+	* glom/libglom/data_structure/translatable_item.[h|cc]:
+	get_title_translation(), get_title_or_name_original():
+	Make these non-virtual, because nothing needs to override them.
+	* glom/libglom/data_structure/layout/layoutitem_field.[h|cc]:
+  Remove useless get_title_original(), get_title_translation(), and
+  get_title_or_name_original() overrides.
+  Remove useless and unused private get_title_no_custom_original() method.
+	* glom/libglom/document/document.[h|cc]: Add a 
+  fill_translatable_layout_items(LayoutItem_Field) method, and use it in 
+  fill_translatable_layout_items(LayoutGroup), instead of repeating code.
+  Avoid calling get_title_original() and get_title_translation() on 
+  LayoutItem_Field because it does not have its own title.
+	* glom/libglom/example_document_load.cc:
+	* tests/test_document_load_translations.cc: Adapted.
+
+2012-01-16  Murray Cumming  <murrayc murrayc com>
+
 	TranslatableItem: Require the caller to provide the locale.
 
 	* glom/libglom/data_structure/translatable_item.[h|cc]:
diff --git a/glom/libglom/data_structure/layout/layoutitem_field.cc b/glom/libglom/data_structure/layout/layoutitem_field.cc
index da2adb4..3ec9777 100644
--- a/glom/libglom/data_structure/layout/layoutitem_field.cc
+++ b/glom/libglom/data_structure/layout/layoutitem_field.cc
@@ -137,18 +137,6 @@ Glib::ustring LayoutItem_Field::get_title_no_custom_translation(const Glib::ustr
     return Glib::ustring();
 }
 
-Glib::ustring LayoutItem_Field::get_title_no_custom_original() const
-{
-  //Use the field's default title:
-  if(m_field_cache_valid && m_field)
-  {
-    return m_field->get_title_original();
-  }
-  else
-    return get_name(); //We ignore TranslatableItem::get_title() for LayoutItem_Field.
-}
-
-
 Glib::ustring LayoutItem_Field::get_title_or_name_no_custom(const Glib::ustring& locale) const
 {
   //Use the field's default title:
@@ -173,32 +161,6 @@ Glib::ustring LayoutItem_Field::get_title(const Glib::ustring& locale) const
   return get_title_no_custom(locale);
 }
 
-Glib::ustring LayoutItem_Field::get_title_translation(const Glib::ustring& locale, bool fallback) const
-{
-  //Use the custom title (overriding the field's default title), if there is one:
-  //This may even be empty if the developer specifies that.
-  if(m_title_custom && m_title_custom->get_use_custom_title())
-  {
-    return m_title_custom->get_title_translation(locale, fallback);
-  }
-
-  return m_field->get_title_translation(locale, fallback);
-}
-
-Glib::ustring LayoutItem_Field::get_title_original() const
-{
-  //Use the custom title (overriding the field's default title), if there is one:
-  //This may even be empty if the developer specifies that.
-  if(m_title_custom && m_title_custom->get_use_custom_title())
-  {
-    return m_title_custom->get_title_original();
-  }
-
-  //Use the field's default title:
-  return get_title_no_custom_original();
-}
-
-
 Glib::ustring LayoutItem_Field::get_title_or_name(const Glib::ustring& locale) const
 {
   //Use the custom title (overriding the field's default title), if there is one:
@@ -212,19 +174,6 @@ Glib::ustring LayoutItem_Field::get_title_or_name(const Glib::ustring& locale) c
   return get_title_no_custom(locale);
 }
 
-Glib::ustring LayoutItem_Field::get_title_or_name_original() const
-{
-  //Use the custom title (overriding the field's default title), if there is one:
-  //This may even be empty if the developer specifies that.
-  if(m_title_custom && m_title_custom->get_use_custom_title())
-  {
-    return m_title_custom->get_title_original();
-  }
-
-  //Use the field's default title:
-  return get_title_no_custom_original();
-}
-
 bool LayoutItem_Field::get_editable_and_allowed() const
 {
   //The relationship might forbid editing of any fields through itself:
diff --git a/glom/libglom/data_structure/layout/layoutitem_field.h b/glom/libglom/data_structure/layout/layoutitem_field.h
index 49a64af..7c4a985 100644
--- a/glom/libglom/data_structure/layout/layoutitem_field.h
+++ b/glom/libglom/data_structure/layout/layoutitem_field.h
@@ -59,6 +59,10 @@ private:
  * The field may be in a known table, or in a to table of a relationship 
  * or related relatinoship. See UsesRelationship::get_relationship() and 
  * UsesRelationship::get_related_relationship() in the base class.
+ *
+ * get_title() returns either the title of the Field or the CustomTitle.
+ * You should not call get/set_title_original() or get/set_title_translation()
+ * on items of this type.
  */
 class LayoutItem_Field 
  : public LayoutItem_WithFormatting,
@@ -89,20 +93,9 @@ public:
   virtual Glib::ustring get_title(const Glib::ustring& locale) const;
 
   /** Get the user-visible title for the field, in the user's current locale.
-   * This returns the name if no title is set.
-   */
-  virtual Glib::ustring get_title_original() const;
-
-  virtual Glib::ustring get_title_translation(const Glib::ustring& locale, bool fallback = true) const;
-
-  /** Get the user-visible title for the field, in the user's current locale.
    */
   virtual Glib::ustring get_title_or_name(const Glib::ustring& locale) const;
 
-  /** Get the user-visible title for the field, in the original locale.
-   */
-  virtual Glib::ustring get_title_or_name_original() const;
-
   Glib::ustring get_title_or_name_no_custom(const Glib::ustring& locale) const;
 
   sharedptr<const CustomTitle> get_title_custom() const;
@@ -176,8 +169,6 @@ private:
 
   Glib::ustring get_title_no_custom(const Glib::ustring& locale) const;
 
-  Glib::ustring get_title_no_custom_original() const;
-
   Glib::ustring get_title_no_custom_translation(const Glib::ustring& locale, bool fallback = true) const;
 
 
diff --git a/glom/libglom/data_structure/layout/layoutitem_portal.h b/glom/libglom/data_structure/layout/layoutitem_portal.h
index 68cd2b0..07f1d70 100644
--- a/glom/libglom/data_structure/layout/layoutitem_portal.h
+++ b/glom/libglom/data_structure/layout/layoutitem_portal.h
@@ -31,6 +31,11 @@ namespace Glom
 
 class Document; //For the utility functions.
 
+/**
+ * get_title() returns either the title of the Field or the CustomTitle.
+ * You should not call get/set_title_original() or get/set_title_translation()
+ * on items of this type.
+ */
 class LayoutItem_Portal
 : public LayoutGroup,
   public UsesRelationship
@@ -48,9 +53,7 @@ public:
   virtual LayoutItem* clone() const;
 
   virtual Glib::ustring get_title(const Glib::ustring& locale) const;
-  //TODO: get_title_original() and get_title_translation()? But this just uses the Relationship title anyway.
   virtual Glib::ustring get_title_or_name(const Glib::ustring& locale) const;
-  //TODO: get_title_or_name_original()? But this just uses the Relationship title anyway.
   virtual Glib::ustring get_part_type_name() const;
 
   virtual void change_field_item_name(const Glib::ustring& table_name, const Glib::ustring& field_name, const Glib::ustring& field_name_new);
diff --git a/glom/libglom/data_structure/layout/report_parts/layoutitem_fieldsummary.h b/glom/libglom/data_structure/layout/report_parts/layoutitem_fieldsummary.h
index b8164d4..eb1cae0 100644
--- a/glom/libglom/data_structure/layout/report_parts/layoutitem_fieldsummary.h
+++ b/glom/libglom/data_structure/layout/report_parts/layoutitem_fieldsummary.h
@@ -62,9 +62,7 @@ public:
   void set_field(const sharedptr<LayoutItem_Field>& field);
 
   virtual Glib::ustring get_title(const Glib::ustring& locale) const;
-  //TODO: get_title_original() and get_title_translation(): But this just uses the field title anyway.
   virtual Glib::ustring get_title_or_name(const Glib::ustring& locale) const;
-  //TODO: get_title_or_name_original()? But this just uses the field title anyway.
 
   virtual Glib::ustring get_layout_display_name() const;
 
diff --git a/glom/libglom/data_structure/translatable_item.cc b/glom/libglom/data_structure/translatable_item.cc
index e5f51ef..225b2b0 100644
--- a/glom/libglom/data_structure/translatable_item.cc
+++ b/glom/libglom/data_structure/translatable_item.cc
@@ -276,13 +276,4 @@ Glib::ustring TranslatableItem::get_title_or_name(const Glib::ustring& locale) c
     return title;
 }
 
-Glib::ustring TranslatableItem::get_title_or_name_original() const
-{
-  const Glib::ustring title = get_title_original();
-  if(title.empty())
-    return get_name();
-  else
-    return title;
-}
-
 } //namespace Glom
diff --git a/glom/libglom/data_structure/translatable_item.h b/glom/libglom/data_structure/translatable_item.h
index 97c84ff..dcfe849 100644
--- a/glom/libglom/data_structure/translatable_item.h
+++ b/glom/libglom/data_structure/translatable_item.h
@@ -53,7 +53,7 @@ public:
 
   virtual Glib::ustring get_title_or_name(const Glib::ustring& locale) const;
 
-  virtual Glib::ustring get_title_or_name_original() const;
+  Glib::ustring get_title_or_name_original() const;
 
   /** Get the title's translation for the specified locale, falling back to the
    * original text if there is no translation.
@@ -66,6 +66,7 @@ public:
    */
   virtual Glib::ustring get_title(const Glib::ustring& locale) const;
 
+  //This is virtual so that ChoiceValue can override it.
   /** Get the title's original (non-translated, usually English) text.
    */
   virtual Glib::ustring get_title_original() const;
@@ -75,7 +76,7 @@ public:
    * the original.
    * Calling this with the current locale is the same as calling get_title_original().
    */
-  virtual Glib::ustring get_title_translation(const Glib::ustring& locale, bool fallback = true) const;
+  Glib::ustring get_title_translation(const Glib::ustring& locale, bool fallback = true) const;
   
 
   /** Set the title's translation for the specified locale.
diff --git a/glom/libglom/document/document.cc b/glom/libglom/document/document.cc
index 458bd6c..86bd92f 100644
--- a/glom/libglom/document/document.cc
+++ b/glom/libglom/document/document.cc
@@ -4443,7 +4443,7 @@ Document::type_list_translatables Document::get_translatable_items()
     const Glib::ustring table_name = tableinfo->get_name();
 
     //The table's field titles:
-    Glib::ustring hint = "Parent table: " + table_name;
+    const Glib::ustring hint = "Parent table: " + table_name;
     type_vec_fields fields = get_table_fields(table_name);
     for(type_vec_fields::iterator iter = fields.begin(); iter != fields.end(); ++iter)
     {
@@ -4543,10 +4543,37 @@ void Document::fill_translatable_custom_choices(FieldFormatting& formatting, typ
   }
 }
 
-void Document::fill_translatable_layout_items(const sharedptr<LayoutGroup>& group, type_list_translatables& the_list, const Glib::ustring& hint)
+void Document::fill_translatable_layout_items(const sharedptr<LayoutItem_Field>& layout_field, type_list_translatables& the_list, const Glib::ustring& hint)
 {
-  the_list.push_back( pair_translatable_item_and_hint(group, hint) );
+  //LayoutItem_Field items do not have their own titles.
+  //They use either the field's title or a custom title:
+  sharedptr<CustomTitle> custom_title = layout_field->get_title_custom();
+  if(custom_title)
+  {
+    the_list.push_back( pair_translatable_item_and_hint(custom_title, hint) ); 
+  }
+
+  //The field will be added separately.
+  
+  //Custom Choices, if any:
+  //Only text fields can have translated choice values:
+  if(layout_field->get_glom_type() == Field::TYPE_TEXT)
+  {
+    const Glib::ustring choice_hint = hint + ", Parent Field: " + layout_field->get_name();
+    fill_translatable_custom_choices(layout_field->m_formatting, the_list, hint);
+  }
+}
+
 
+void Document::fill_translatable_layout_items(const sharedptr<LayoutGroup>& group, type_list_translatables& the_list, const Glib::ustring& hint)
+{
+  //Portals don't have their own titles - they use the relationship title (though we might want to allow custom titles)
+  sharedptr<LayoutItem_Portal> portal = sharedptr<LayoutItem_Portal>::cast_dynamic(group);
+  if(!portal)
+  {
+    the_list.push_back( pair_translatable_item_and_hint(group, hint) );
+  }
+  
   const Glib::ustring group_name = group->get_name();
   Glib::ustring this_hint = hint;
   if(!group_name.empty())
@@ -4565,11 +4592,7 @@ void Document::fill_translatable_layout_items(const sharedptr<LayoutGroup>& grou
       if(group_by)
       {
         sharedptr<LayoutItem_Field> field = group_by->get_field_group_by();
-        sharedptr<CustomTitle> custom_title = field->get_title_custom();
-        if(custom_title)
-        {
-          the_list.push_back( pair_translatable_item_and_hint(custom_title, this_hint) ); 
-        }
+        fill_translatable_layout_items(field, the_list, hint);
 
         fill_translatable_layout_items(group_by->m_group_secondary_fields, the_list, this_hint);
       }
@@ -4588,17 +4611,7 @@ void Document::fill_translatable_layout_items(const sharedptr<LayoutGroup>& grou
         sharedptr<LayoutItem_Field> layout_field = sharedptr<LayoutItem_Field>::cast_dynamic(item);
         if(layout_field)
         {
-          sharedptr<CustomTitle> custom_title = layout_field->get_title_custom();
-          if(custom_title)
-          {
-            the_list.push_back( pair_translatable_item_and_hint(custom_title, this_hint) ); 
-          }
-
-          //Custom Choices, if any:
-          //Only text fields can have translated choice values:
-          const Glib::ustring choice_hint = this_hint + ", Parent Field: " + layout_field->get_name();
-          if(layout_field->get_glom_type() == Field::TYPE_TEXT)
-            fill_translatable_custom_choices(layout_field->m_formatting, the_list, this_hint);
+          fill_translatable_layout_items(layout_field, the_list, hint);
         }
       }
     }
diff --git a/glom/libglom/document/document.h b/glom/libglom/document/document.h
index 6a32e14..237b823 100644
--- a/glom/libglom/document/document.h
+++ b/glom/libglom/document/document.h
@@ -523,6 +523,7 @@ private:
 
   void on_app_state_userlevel_changed(AppState::userlevels userlevel);
 
+  static void fill_translatable_layout_items(const sharedptr<LayoutItem_Field>& layout_field, type_list_translatables& the_list, const Glib::ustring& hint);
   static void fill_translatable_layout_items(const sharedptr<LayoutGroup>& group, type_list_translatables& the_list, const Glib::ustring& hint);
 
   void fill_sort_field_details(const Glib::ustring& parent_table_name, FieldFormatting::type_list_sort_fields& sort_fields) const;
diff --git a/glom/libglom/example_document_load.cc b/glom/libglom/example_document_load.cc
index c676227..adb8d82 100644
--- a/glom/libglom/example_document_load.cc
+++ b/glom/libglom/example_document_load.cc
@@ -40,7 +40,7 @@ void print_layout_group(const Glom::sharedptr<Glom::LayoutGroup>& layout_group,
     if(!layout_item)
       continue;
 
-    std::cout << indent << "Layout Item: title=" << layout_item->get_title_or_name_original()
+    std::cout << indent << "Layout Item: title=" << layout_item->get_title_or_name(Glib::ustring() /* locale */)
       << ", item type=" << layout_item->get_part_type_name();
 
     const Glib::ustring display_name = layout_item->get_layout_display_name();
@@ -66,7 +66,7 @@ void print_layout(const Glom::Document::type_list_layout_groups& layout_groups)
     if(!layout_group)
       continue;
 
-    std::cout << "    Layout Group: title=" << layout_group->get_title_or_name_original() << std::endl;
+    std::cout << "    Layout Group: title=" << layout_group->get_title_or_name(Glib::ustring() /* locale */) << std::endl;
     print_layout_group(layout_group, "      ");
   }
 }
@@ -125,7 +125,7 @@ int main()
        const Glom::Field::glom_field_type field_type = field->get_glom_type();
 
        std::cout << "  Field: name=" << field->get_name()
-         << ", title=" << field->get_title_or_name_original()
+         << ", title=" << field->get_title_or_name(Glib::ustring() /* locale */)
          << ", type=" << Glom::Field::get_type_name_ui(field_type) << std::endl;
 
     }
diff --git a/tests/test_document_load_translations.cc b/tests/test_document_load_translations.cc
index 5c8aa3c..00ed02f 100644
--- a/tests/test_document_load_translations.cc
+++ b/tests/test_document_load_translations.cc
@@ -162,30 +162,43 @@ void check_title(const T_Item& item, const char* title_en, const char* title_de)
 {
   g_assert(item);
 
-  g_assert( item->get_title_original() == title_en );
+  //The get_title_original() and get_title_translation() should not be called 
+  //on items that delegate to a child item:
+  bool has_own_title = true;
+  Glom::sharedptr<const Glom::LayoutItem_Field> field = 
+    Glom::sharedptr<const Glom::LayoutItem_Field>::cast_dynamic(item);
+  if(field)
+     has_own_title = false;
+
+  if(has_own_title)
+    g_assert( item->get_title_original() == title_en );
+    
   g_assert( item->get_title(Glib::ustring()) == title_en );
   g_assert( item->get_title("en_US") == title_en );
 
-  g_assert( item->get_title_translation(locale_de) == title_de );
+  if(has_own_title)
+    g_assert( item->get_title_translation(locale_de) == title_de );
+    
   g_assert( item->get_title(locale_de) == title_de );
 
-  g_assert( item->get_title_or_name_original() == title_en );
   g_assert( item->get_title_or_name(Glib::ustring()) == title_en );
   g_assert( item->get_title_or_name("en_US") == title_en );
   g_assert( item->get_title_or_name(locale_de) == title_de );
 
-
-  //Check fallbacks:
-  g_assert( item->get_title_translation(Glib::ustring()) == title_en );
-  g_assert( item->get_title_translation("en_US") == title_en );
-  g_assert( item->get_title_translation("en_GB") == title_en );
-  g_assert( item->get_title_translation("de_AU") == title_de );
-
-  //Check that fallbacks do not happen when we don't want them:
-  g_assert( item->get_title_translation(Glib::ustring(), false) == Glib::ustring() );
-  g_assert( item->get_title_translation("en_US", false) == Glib::ustring() );
-  g_assert( item->get_title_translation("en_GB", false) == Glib::ustring() );
-  g_assert( item->get_title_translation("de_AU", false) == Glib::ustring() );
+  if(has_own_title)
+  {
+    //Check fallbacks:
+    g_assert( item->get_title_translation(Glib::ustring()) == title_en );
+    g_assert( item->get_title_translation("en_US") == title_en );
+    g_assert( item->get_title_translation("en_GB") == title_en );
+    g_assert( item->get_title_translation("de_AU") == title_de );
+
+    //Check that fallbacks do not happen when we don't want them:
+    g_assert( item->get_title_translation(Glib::ustring(), false) == Glib::ustring() );
+    g_assert( item->get_title_translation("en_US", false) == Glib::ustring() );
+    g_assert( item->get_title_translation("en_GB", false) == Glib::ustring() );
+    g_assert( item->get_title_translation("de_AU", false) == Glib::ustring() );
+  }
 }
 
 int main()
@@ -277,7 +290,7 @@ int main()
   Glom::sharedptr<const Glom::LayoutItem_Field> field_on_layout = 
     get_field_on_layout(document, "characters", "contacts", "name_full");
   g_assert(field_on_layout);
-  check_title(field_on_layout, "Actor's Name", "Schauspieler Name" );
+  check_title(field_on_layout, "Actor's Name", "Schauspieler Name");
 
   //Check a LayoutItemField's Field title:
   field_on_layout =
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]