LT-21205: Features should obey the order set in Feature Types (#319)

* LT-21205: Sort features in LongName and make LongName and LongNameSorted use the same code

Author: jtmaxwell3

src/SIL.LCModel/DomainImpl/OverridesCellar.cs

@@ -2460,7 +2460,7 @@ public ITsString LongNameSortedTSS
 		{
 			get
 			{
-				return GetFeatureValueStringSorted();
+				return GetFeatureValueString(true);
 			}
 		}
 		/// <summary>
@@ -2494,17 +2494,6 @@ private ITsString GetFeatureValueString(bool fLongForm)
 			return tisb.GetString();
 		}
 
-		private ITsString GetFeatureValueStringSorted()
-		{
-			var tisb = TsStringUtils.MakeIncStrBldr();
-			tisb.SetIntPropValues((int)FwTextPropType.ktptWs,
-				0, Cache.DefaultAnalWs);
-			var sFeature = GetFeatureString(true);
-			tisb.Append(sFeature);
-			tisb.AppendTsString((ValueOA as FsFeatStruc).GetFeatureValueStringSorted());
-			return tisb.GetString();
-		}
-
 		private string GetValueString(bool fLongForm)
 		{
 			var sValue = "";
@@ -3794,7 +3783,7 @@ public ITsString LongNameSortedTSS
 		{
 			get
 			{
-				return GetFeatureValueStringSorted();
+				return GetFeatureValueString(true);
 			}
 		}
 		/// <summary>
@@ -3947,7 +3936,10 @@ private ITsString GetFeatureValueString(bool fLongForm)
 				tisb.Append("[");
 			}
 			var count = 0;
-			foreach (var spec in FeatureSpecsOC)
+			var sortedSpecs = from s in FeatureSpecsOC
+							  orderby FeatureSpecKey(s)
+							  select s;
+			foreach (var spec in sortedSpecs)
 			{
 				if (count++ > 0)
 				{
@@ -3994,56 +3986,25 @@ private ITsString GetFeatureValueString(bool fLongForm)
 			return tisb.GetString();
 		}
 
-		internal ITsString GetFeatureValueStringSorted()
+		private string FeatureSpecKey(IFsFeatureSpecification spec)
 		{
-			var tisb = TsStringUtils.MakeIncStrBldr();
-			var iCount = FeatureSpecsOC.Count;
-			if (iCount > 0)
+			// specs with features in the type go first (because they begin with 0),
+			// then specs with features (because they begin with 1),
+			// then specs without features (because they begin with 2).
+			if (spec.FeatureRA == null)
+				return "2" + spec.Guid.ToString();
+			if (TypeRA != null)
 			{
-				tisb.SetIntPropValues((int)FwTextPropType.ktptWs, (int)FwTextPropVar.ktpvDefault,
-					m_cache.DefaultAnalWs);
-				tisb.Append("[");
-			}
-			var count = 0;
-			var sortedSpecs = from s in FeatureSpecsOC
-							  orderby s.FeatureRA.Name.BestAnalysisAlternative.Text
-							  select s;
-			foreach (var spec in sortedSpecs)
-			{
-				if (count++ > 0)
-				{
-					tisb.SetIntPropValues((int)FwTextPropType.ktptWs, (int)FwTextPropVar.ktpvDefault,
-						m_cache.DefaultAnalWs);
-					tisb.Append(" "); // insert space except for first item
-				}
-				var cv = spec as IFsClosedValue;
-				if (cv != null)
-				{
-						tisb.AppendTsString((cv as FsClosedValue).LongNameTSS);
-				}
-				else
+				int index = TypeRA.FeaturesRS.IndexOf(spec.FeatureRA);
+				if (index >= 0)
 				{
-					var complex = spec as IFsComplexValue;
-					if (complex != null)
-					{
-							tisb.AppendTsString((complex as FsComplexValue).LongNameSortedTSS);
-					}
+					// This won't work properly if there are more than 999 features in TypeRA.
+					return index.ToString("0000");
 				}
 			}
-			if (iCount > 0)
-			{
-				tisb.SetIntPropValues((int)FwTextPropType.ktptWs, (int)FwTextPropVar.ktpvDefault,
-					m_cache.DefaultAnalWs);
-				tisb.Append("]");
-			}
-			if (tisb.Text == null)
-			{
-				// Ensure that we have a ws for the empty string!  See FWR-1122.
-				tisb.SetIntPropValues((int)FwTextPropType.ktptWs, (int)FwTextPropVar.ktpvDefault,
-					m_cache.DefaultAnalWs);
-			}
-			return tisb.GetString();
+			return "1" + spec.FeatureRA.Name.BestAnalysisAlternative.Text;
 		}
+
 		/// <summary>
 		/// Provide a "Name" for this (is a long name)
 		/// </summary>

tests/SIL.LCModel.Tests/DomainImpl/CellarTests.cs

@@ -640,7 +640,7 @@ public void AddComplexFeaturesToFeatureSystemAndThenToAFeatureStructure()
 			pos.AddInflectableFeatsFromXml(itemAorist);
 			featStruct.AddFeatureFromXml(itemAorist, msfs);
 			// Check for correct LongName
-			Assert.AreEqual("[sbj:[gen:f pers:1] asp:aor]", featStruct.LongName, "Incorrect LongName for complex and closed");
+			Assert.AreEqual("[asp:aor sbj:[gen:f pers:1]]", featStruct.LongName, "Incorrect LongName for complex and closed");
 			// Now add the features in the featurs struct in a different order
 			pos.DefaultFeaturesOA = null;
 			pos.DefaultFeaturesOA = Cache.ServiceLocator.GetInstance<IFsFeatStrucFactory>().Create();
@@ -649,9 +649,9 @@ public void AddComplexFeaturesToFeatureSystemAndThenToAFeatureStructure()
 			featStruct.AddFeatureFromXml(item1st, msfs);
 			featStruct.AddFeatureFromXml(itemFem, msfs);
 			// check for correct short name
-			Assert.AreEqual("aor 1 f", featStruct.ShortName, "Incorrect ShortName for complex");
+			Assert.AreEqual("aor f 1", featStruct.ShortName, "Incorrect ShortName for complex");
 			// Check for correct LongName
-			Assert.AreEqual("[asp:aor sbj:[pers:1 gen:f]]", featStruct.LongName, "Incorrect LongName for complex and closed");
+			Assert.AreEqual("[asp:aor sbj:[gen:f pers:1]]", featStruct.LongName, "Incorrect LongName for complex and closed");
 			// Now create another feature structure with different values and merge it into the first feature structure
 			pos.InherFeatValOA = Cache.ServiceLocator.GetInstance<IFsFeatStrucFactory>().Create();
 			IFsFeatStruc featStruct2 = pos.InherFeatValOA;
@@ -665,7 +665,7 @@ public void AddComplexFeaturesToFeatureSystemAndThenToAFeatureStructure()
 			featStruct2.AddFeatureFromXml(itemSg, msfs);
 			featStruct.PriorityUnion(featStruct2);
 			// Check for correct LongName
-			Assert.AreEqual("[asp:aor sbj:[pers:1 gen:n num:sg]]", featStruct.LongName, "Incorrect LongName for merged feature struture");
+			Assert.AreEqual("[asp:aor sbj:[gen:n num:sg pers:1]]", featStruct.LongName, "Incorrect LongName for merged feature struture");
 			Assert.AreEqual("[asp:aor sbj:[gen:n num:sg pers:1]]", featStruct.LongNameSorted, "Incorrect LongNameSorted for merged feature struture");
 		}