Sort writing systems retrieved from import LDMLs (#2349)

.vscode/settings.json

@@ -60,6 +60,7 @@
     "reportgenerator",
     "sillsdev",
     "Sldr",
+    "Subtag",
     "targetdir",
     "textfile",
     "thecombine",

Backend.Tests/Helper/LanguageTests.cs

@@ -38,13 +38,32 @@ public void TestGetSenseAnalysisLangTagsFull()
         public void TestConvertLangTagToWritingSystemEn()
         {
             var tags = new List<string> { "en", "en-US", "ajsdlfj" };
-            var writingSystems = new List<WritingSystem> {
-                new WritingSystem {Bcp47 = "en", Name = "English"},
-                new WritingSystem {Bcp47 = "en-US", Name = "English"},
-                new WritingSystem {Bcp47 = "ajsdlfj", Name = ""},
-
-            };
+            var writingSystems =
+                new List<WritingSystem> { new("en", "English"), new("en-US", "English"), new("ajsdlfj") };
             Assert.That(ConvertLangTagsToWritingSystems(tags).ToList(), Is.EqualTo(writingSystems));
         }
+
+        [Test]
+        public void TestCompareWritingSystems()
+        {
+            // 3-letter codes first.
+            //      Named before unnamed, alphabetical primarily by name.
+            var ws1 = new WritingSystem("pis", "Pijin");
+            var ws2 = new WritingSystem("pis", "Solomons Pijin");
+            //      Unnamed after named, alphabetical by code.
+            var ws3 = new WritingSystem("pis-x-PIJ");
+            var ws4 = new WritingSystem("qaa-pis");
+            // 2-letter codes last.
+            //      Named before unnamed, alphabetical secondarily by code.
+            var ws5 = new WritingSystem("en", "English");
+            var ws6 = new WritingSystem("en-US", "English");
+            //      Unnamed after named.
+            var ws7 = new WritingSystem("en");
+
+            var toSort = new List<WritingSystem> { ws6, ws7, ws4, ws5, ws2, ws3, ws1 };
+            toSort.Sort(CompareWritingSystems);
+            Assert.That(toSort, Is.EqualTo(
+                new List<WritingSystem> { ws1, ws2, ws3, ws4, ws5, ws6, ws7 }));
+        }
     }
 }

Backend.Tests/Models/ProjectTests.cs

@@ -99,7 +99,7 @@ public void TestNotEquals()
         [Test]
         public void TestClone()
         {
-            var system = new WritingSystem { Name = "WritingSystemName", Bcp47 = "en", Font = "calibri" };
+            var system = new WritingSystem("en", "WritingSystemName", "calibri");
             var project = new Project { Name = "ProjectName", VernacularWritingSystem = system };
             var domain = new SemanticDomain { Name = "SemanticDomainName", Id = "1" };
             project.SemanticDomains.Add(domain);
@@ -170,43 +170,46 @@ public void TestHashCode()
 
     public class WritingSystemTests
     {
-        private const string Name = "System 1";
         private const string Bcp47 = "lang-1";
+        private const string Name = "System 1";
         private const string Font = "calibri";
 
         [Test]
         public void TestEquals()
         {
-            var system = new WritingSystem { Name = Name };
-            Assert.That(system.Equals(new WritingSystem { Name = Name }));
+            var system = new WritingSystem(Bcp47, Name);
+            Assert.That(system.Equals(new WritingSystem(Bcp47, Name)));
         }
 
         [Test]
         public void TestEqualsNull()
         {
-            var system = new WritingSystem { Name = Name };
+            var system = new WritingSystem(Bcp47, Name);
             Assert.IsFalse(system.Equals(null));
         }
 
         [Test]
         public void TestNotEquals()
         {
-            var system = new WritingSystem { Name = Name, Bcp47 = Bcp47 };
-            Assert.IsFalse(system.Equals(new WritingSystem { Name = Name }));
+            var system = new WritingSystem(Bcp47, Name);
+            Assert.IsFalse(system.Equals(new WritingSystem(Bcp47)));
+
+            system = new WritingSystem(Bcp47, Name, Font);
+            Assert.IsFalse(system.Equals(new WritingSystem(Bcp47, Name)));
         }
 
         [Test]
         public void TestToString()
         {
-            var system = new WritingSystem { Name = Name, Bcp47 = Bcp47 };
+            var system = new WritingSystem(Bcp47, Name);
             var sysString = system.ToString();
             Assert.IsTrue(sysString.Contains(Name) && sysString.Contains(Bcp47));
         }
 
         [Test]
         public void TestClone()
         {
-            var system = new WritingSystem { Name = Name, Bcp47 = Bcp47, Font = Font };
+            var system = new WritingSystem(Bcp47, Name, Font);
             var clonedSystem = system.Clone();
             Assert.AreEqual(system, clonedSystem);
         }

Backend.Tests/Util.cs

@@ -132,12 +132,7 @@ public static Project RandomProject()
 
         public static WritingSystem RandomWritingSystem()
         {
-            return new WritingSystem
-            {
-                Name = RandString(),
-                Bcp47 = RandString(),
-                Font = RandString()
-            };
+            return new(RandString(), RandString(), RandString());
         }
     }
 }

Backend/Controllers/LiftController.cs

@@ -74,7 +74,7 @@ internal async Task<IActionResult> UploadLiftFileAndGetWritingSystems(FileUpload
                 return BadRequest(e.Message);
             }
 
-            return Ok(Language.GetWritingSystems(extractedLiftRootPath).ToList());
+            return Ok(Language.GetWritingSystems(extractedLiftRootPath));
         }
 
         /// <summary> Adds data from a directory containing a .lift file </summary>
@@ -106,7 +106,7 @@ internal async Task<IActionResult> FinishUploadLiftFile(string projectId, string
             }
 
             var extractDir = _liftService.RetrieveImport(userId);
-            if (String.IsNullOrWhiteSpace(extractDir))
+            if (string.IsNullOrWhiteSpace(extractDir))
             {
                 return BadRequest("No in-progress import to finish.");
             }
@@ -238,11 +238,11 @@ private async Task<IActionResult> AddImportToProject(string liftStoragePath, str
             // Add analysis writing systems found in the data, avoiding duplicate and empty bcp47 codes.
             project.AnalysisWritingSystems.AddRange(importedAnalysisWritingSystems.Where(
                 iws => !project.AnalysisWritingSystems.Any(ws => ws.Bcp47 == iws.Bcp47)));
-            project.AnalysisWritingSystems.RemoveAll(ws => String.IsNullOrWhiteSpace(ws.Bcp47));
+            project.AnalysisWritingSystems.RemoveAll(ws => string.IsNullOrWhiteSpace(ws.Bcp47));
             if (project.AnalysisWritingSystems.Count == 0)
             {
                 // The list cannot be empty.
-                project.AnalysisWritingSystems.Add(new WritingSystem { Bcp47 = "en", Name = "English" });
+                project.AnalysisWritingSystems.Add(new("en", "English"));
             }
 
             // Store whether we have imported any senses with definitions or grammatical info

Backend/Helper/GrammaticalCategory.cs

@@ -155,7 +155,7 @@ public bool Matches(string gramCat)
         public static GramCatGroup GetGramCatGroup(string grammaticalCategory)
         {
             var gramCat = removeAccents(grammaticalCategory);
-            if (String.IsNullOrWhiteSpace(Regex.Match(gramCat, @"[^\W]+").Value))
+            if (string.IsNullOrWhiteSpace(Regex.Match(gramCat, @"[^\W]+").Value))
             {
                 return GramCatGroup.Unspecified;
             }

Backend/Helper/Language.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -11,15 +12,15 @@ public static class Language
         /// <summary>
         /// Extract list of distinct Language strings from all of a <see cref="Sense"/>'s Definitions and Glosses.
         /// </summary>
-        public static IEnumerable<string> GetSenseAnalysisLangTags(Sense sense)
+        public static List<string> GetSenseAnalysisLangTags(Sense sense)
         {
             var tags = sense.Definitions.Select(d => d.Language).ToList();
             tags.AddRange(sense.Glosses.Select(g => g.Language));
-            return tags.Distinct();
+            return tags.Distinct().ToList();
         }
 
         /// <summary> Convert language tags into writing systems. </summary>
-        public static IEnumerable<WritingSystem> ConvertLangTagsToWritingSystems(IEnumerable<string> langTags)
+        public static List<WritingSystem> ConvertLangTagsToWritingSystems(IEnumerable<string> langTags)
         {
             // If Sldr isn't initialized, temporarily initialize it here.
             var isSldrInitialized = Sldr.IsInitialized;
@@ -29,26 +30,60 @@ public static IEnumerable<WritingSystem> ConvertLangTagsToWritingSystems(IEnumer
             }
             var langLookup = new LanguageLookup();
             var writingSystems = langTags.Select(tag =>
-                new WritingSystem { Bcp47 = tag, Name = langLookup.GetLanguageFromCode(tag)?.DesiredName ?? "" });
+                new WritingSystem(tag, langLookup.GetLanguageFromCode(tag)?.DesiredName ?? ""));
             if (!isSldrInitialized)
             {
                 Sldr.Cleanup();
             }
-            return writingSystems;
+            return writingSystems.ToList();
         }
 
         /// <summary>
         /// Extract <see cref="WritingSystem"/>s from .ldml files in a directory or its WritingSystems subdirectory.
         /// </summary>
-        public static IEnumerable<WritingSystem> GetWritingSystems(string dirPath)
+        public static List<WritingSystem> GetWritingSystems(string dirPath)
         {
             if (!Directory.GetFiles(dirPath, "*.ldml").Any())
             {
                 dirPath = Path.Combine(dirPath, "WritingSystems");
             }
 
             var wsr = LdmlInFolderWritingSystemRepository.Initialize(dirPath);
-            return wsr.AllWritingSystems.Select(wsd => new WritingSystem(wsd));
+            var ws = wsr.AllWritingSystems.Select(wsd => new WritingSystem(wsd)).ToList();
+            ws.Sort(CompareWritingSystems);
+            return ws;
+        }
+
+        /// <summary> A comparison function for sorting a List of writing systems. </summary>
+        public static int CompareWritingSystems(WritingSystem x, WritingSystem y)
+        {
+            var xTag = x.Bcp47;
+            var yTag = y.Bcp47;
+            var xSubtag = xTag.Split("-").First();
+            var ySubtag = yTag.Split("-").First();
+            if (xSubtag.Length != ySubtag.Length)
+            {
+                // 3-letter language tags should sort before 2-letter ones.
+                return ySubtag.Length - xSubtag.Length;
+            }
+            if (x.Name != y.Name)
+            {
+                // Named writing systems should sort before nameless ones.
+                if (string.IsNullOrEmpty(x.Name))
+                {
+                    return 1;
+                }
+                if (string.IsNullOrEmpty(y.Name))
+                {
+                    return -1;
+                }
+
+                // Primarily sort by writing system Name.
+                return String.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase);
+            }
+
+            // Secondarily sort by writing system BCP47 tag.
+            return String.Compare(xTag, yTag, StringComparison.OrdinalIgnoreCase);
         }
     }
 }

Backend/Helper/LiftHelper.cs

@@ -63,7 +63,7 @@ public static string GetLiftRootFromExtractedZip(string dirPath)
         public static bool IsProtected(LiftEntry entry)
         {
             return entry.Annotations.Count > 0 || entry.Etymologies.Count > 0 || entry.Fields.Count > 0 ||
-                (entry.Notes.Count == 1 && !String.IsNullOrEmpty(entry.Notes.First().Type)) ||
+                (entry.Notes.Count == 1 && !string.IsNullOrEmpty(entry.Notes.First().Type)) ||
                 entry.Notes.Count > 1 || entry.Pronunciations.Count > 0 || entry.Relations.Count > 0 ||
                 entry.Traits.Any(t => !t.Value.Equals("stem", StringComparison.OrdinalIgnoreCase)) ||
                 entry.Variants.Count > 0;

Backend/Models/Project.cs

@@ -285,35 +285,30 @@ public override int GetHashCode()
 
     public class WritingSystem
     {
-        [Required]
-        public string Name { get; set; }
         [Required]
         public string Bcp47 { get; set; }
         [Required]
+        public string Name { get; set; }
+        [Required]
         public string Font { get; set; }
 
-        public WritingSystem()
+        public WritingSystem(string bcp47 = "", string name = "", string font = "")
         {
-            Name = "";
-            Bcp47 = "";
-            Font = "";
+            Bcp47 = bcp47;
+            Name = name;
+            Font = font;
         }
 
         public WritingSystem(WritingSystemDefinition wsd)
         {
-            Name = wsd.Language?.Name ?? "";
             Bcp47 = wsd.LanguageTag;
+            Name = wsd.Language?.Name ?? "";
             Font = wsd.DefaultFont?.Name ?? "";
         }
 
         public WritingSystem Clone()
         {
-            return new WritingSystem
-            {
-                Name = Name,
-                Bcp47 = Bcp47,
-                Font = Font
-            };
+            return new(Bcp47, Name, Font);
         }
 
         public override bool Equals(object? obj)
@@ -323,14 +318,14 @@ public override bool Equals(object? obj)
                 return false;
             }
 
-            return Name.Equals(ws.Name, StringComparison.Ordinal) &&
-                Bcp47.Equals(ws.Bcp47, StringComparison.Ordinal) &&
+            return Bcp47.Equals(ws.Bcp47, StringComparison.Ordinal) &&
+                Name.Equals(ws.Name, StringComparison.Ordinal) &&
                 Font.Equals(ws.Font, StringComparison.Ordinal);
         }
 
         public override int GetHashCode()
         {
-            return HashCode.Combine(Name, Bcp47, Font);
+            return HashCode.Combine(Bcp47, Name, Font);
         }
 
         public override string ToString()

Backend/Services/LiftService.cs

@@ -390,7 +390,7 @@ public async Task<string> LiftExport(
             // Export character set to ldml.
             var ldmlDir = Path.Combine(zipDir, "WritingSystems");
             Directory.CreateDirectory(ldmlDir);
-            if (!String.IsNullOrWhiteSpace(proj.VernacularWritingSystem.Bcp47))
+            if (!string.IsNullOrWhiteSpace(proj.VernacularWritingSystem.Bcp47))
             {
                 var validChars = proj.ValidCharacters;
                 LdmlExport(ldmlDir, proj.VernacularWritingSystem, validChars);
@@ -531,7 +531,7 @@ private static void LdmlExport(string filePath, WritingSystem vernacularWS, List
             wsf.Create(vernacularWS.Bcp47, out var wsDef);
 
             // If the vernacular writing system font isn't present, add it.
-            if (!String.IsNullOrWhiteSpace(vernacularWS.Font)
+            if (!string.IsNullOrWhiteSpace(vernacularWS.Font)
                 && !wsDef.Fonts.Any(f => f.Name == vernacularWS.Font))
             {
                 wsDef.Fonts.Add(new FontDefinition(vernacularWS.Font));
@@ -648,7 +648,7 @@ public List<WritingSystem> GetImportAnalysisWritingSystems()
                     w => w.Senses.SelectMany(s => Language.GetSenseAnalysisLangTags(s))
                 ).Distinct();
 
-                return Language.ConvertLangTagsToWritingSystems(langTags).ToList();
+                return Language.ConvertLangTagsToWritingSystems(langTags);
             }
 
             /// <summary>
@@ -771,7 +771,7 @@ public void FinishEntry(LiftEntry entry)
                     }
 
                     // Add grammatical info
-                    if (!String.IsNullOrWhiteSpace(sense.GramInfo?.Value))
+                    if (!string.IsNullOrWhiteSpace(sense.GramInfo?.Value))
                     {
                         newSense.GrammaticalInfo = new GrammaticalInfo(sense.GramInfo.Value);
                     }