From 6075badea0dbfe0eab451913a54bb729a498c39b Mon Sep 17 00:00:00 2001
From: Oz Linden <oz@lindenlab.com>
Date: Fri, 25 May 2012 19:24:38 -0400
Subject: [PATCH] fix problems due to LLSD-30, and implement persistent
 settings

---
 ...ttings_autoreplace.xml => autoreplace.xml} |   0
 indra/newview/llautoreplace.cpp               | 142 ++++++++++++------
 indra/newview/llautoreplace.h                 |  15 +-
 .../newview/llfloaterautoreplacesettings.cpp  |   5 +-
 4 files changed, 105 insertions(+), 57 deletions(-)
 rename indra/newview/app_settings/{settings_autoreplace.xml => autoreplace.xml} (100%)

diff --git a/indra/newview/app_settings/settings_autoreplace.xml b/indra/newview/app_settings/autoreplace.xml
similarity index 100%
rename from indra/newview/app_settings/settings_autoreplace.xml
rename to indra/newview/app_settings/autoreplace.xml
diff --git a/indra/newview/llautoreplace.cpp b/indra/newview/llautoreplace.cpp
index 0432bf735a6..32cd7d2ae3a 100644
--- a/indra/newview/llautoreplace.cpp
+++ b/indra/newview/llautoreplace.cpp
@@ -32,7 +32,7 @@
 
 LLAutoReplace* LLAutoReplace::sInstance;
 
-const char* LLAutoReplace::SETTINGS_FILE_NAME = "settings_autoreplace.xml";
+const char* LLAutoReplace::SETTINGS_FILE_NAME = "autoreplace.xml";
 
 LLAutoReplace::LLAutoReplace()
 {
@@ -144,6 +144,8 @@ LLAutoReplaceSettings LLAutoReplace::getSettings()
 void LLAutoReplace::setSettings(const LLAutoReplaceSettings& newSettings)
 {
 	mSettings.set(newSettings);
+	/// Make the newSettings active and write them to user storage
+	saveToUserSettings();
 }
 
 void LLAutoReplace::loadFromSettings()
@@ -175,10 +177,9 @@ void LLAutoReplace::loadFromSettings()
 	else // no user settings found, try application settings
 	{
 		std::string defaultName = getAppSettingsFileName();
-		LL_INFOS("AutoReplace") << " user settings file '" << filename << "' not found;\n"
-								<< " trying '" << defaultName.c_str() << "'"
-								<< LL_ENDL;
+		LL_INFOS("AutoReplace") << " user settings file '" << filename << "' not found"<< LL_ENDL;
 
+		bool gotSettings = false;
 		if(gDirUtilp->fileExists(defaultName))
 		{
 			LLSD appDefault;
@@ -192,15 +193,16 @@ void LLAutoReplace::loadFromSettings()
 
 			if ( mSettings.setFromLLSD(appDefault) )
 			{
-				LL_INFOS("AutoReplace") << "settings loaded from '" << filename << "'" << LL_ENDL;
-				saveToUserSettings(appDefault);
+				LL_INFOS("AutoReplace") << "settings loaded from '" << defaultName.c_str() << "'" << LL_ENDL;
+				gotSettings = true;
 			}
 			else
 			{
-				LL_WARNS("AutoReplace") << "invalid settings found in '" << filename << "'" << LL_ENDL;
+				LL_WARNS("AutoReplace") << "invalid settings found in '" << defaultName.c_str() << "'" << LL_ENDL;
 			}
 		}
-		else
+		
+		if ( ! gotSettings )
 		{
 			if (mSettings.setFromLLSD(mSettings.getExampleLLSD()))
 			{
@@ -214,12 +216,12 @@ void LLAutoReplace::loadFromSettings()
 	}
 }
 
-void LLAutoReplace::saveToUserSettings(const LLSD& newSettings)
+void LLAutoReplace::saveToUserSettings()
 {
 	std::string filename=getUserSettingsFileName();
 	llofstream file;
 	file.open(filename.c_str());
-	LLSDSerialize::toPrettyXML(newSettings, file);
+	LLSDSerialize::toPrettyXML(mSettings.getAsLLSD(), file);
 	file.close();
 	LL_INFOS("AutoReplace") << "settings saved to '" << filename << "'" << LL_ENDL;
 }
@@ -245,24 +247,27 @@ LLAutoReplaceSettings::LLAutoReplaceSettings(const LLAutoReplaceSettings& settin
 		  list++
 		 )
 	{
-		LLSD listMap = LLSD::emptyMap();
-		std::string listName = (*list)[AUTOREPLACE_LIST_NAME];
-		listMap[AUTOREPLACE_LIST_NAME] = listName;
-		listMap[AUTOREPLACE_LIST_REPLACEMENTS] = LLSD::emptyMap();
-
-		for ( LLSD::map_const_iterator
-				  entry = (*list)[AUTOREPLACE_LIST_REPLACEMENTS].beginMap(),
-				  entriesEnd = (*list)[AUTOREPLACE_LIST_REPLACEMENTS].endMap();
-			  entry != entriesEnd;
-			  entry++
-			 )
+		if ( (*list).isMap() ) // can fail due to LLSD-30: ignore it
 		{
-			std::string keyword = entry->first;
-			std::string replacement = entry->second.asString();
-			listMap[AUTOREPLACE_LIST_REPLACEMENTS].insert(keyword, LLSD(replacement));
-		}
+			LLSD listMap = LLSD::emptyMap();
+			std::string listName = (*list)[AUTOREPLACE_LIST_NAME];
+			listMap[AUTOREPLACE_LIST_NAME] = listName;
+			listMap[AUTOREPLACE_LIST_REPLACEMENTS] = LLSD::emptyMap();
+
+			for ( LLSD::map_const_iterator
+					  entry = (*list)[AUTOREPLACE_LIST_REPLACEMENTS].beginMap(),
+					  entriesEnd = (*list)[AUTOREPLACE_LIST_REPLACEMENTS].endMap();
+				  entry != entriesEnd;
+				  entry++
+				 )
+			{
+				std::string keyword = entry->first;
+				std::string replacement = entry->second.asString();
+				listMap[AUTOREPLACE_LIST_REPLACEMENTS].insert(keyword, LLSD(replacement));
+			}
 
-		mLists.append(listMap);
+			mLists.append(listMap);
+		}
 	}
 }
 
@@ -284,7 +289,10 @@ bool LLAutoReplaceSettings::setFromLLSD(const LLSD& settingsFromLLSD)
 			  list++
 			 )
 		{
-			settingsValid = listIsValid(*list);
+			if ( (*list).isDefined() ) // can be undef due to LLSD-30: ignore it
+			{
+				settingsValid = listIsValid(*list);
+			}
 		}
 	}
 	else
@@ -365,9 +373,11 @@ std::string LLAutoReplaceSettings::replacementFor(std::string keyword, std::stri
 
 LLSD LLAutoReplaceSettings::getListNames()
 {
+	LL_DEBUGS("AutoReplace")<<"====="<<LL_ENDL;
 	LLSD toReturn = LLSD::emptyArray();
+	S32 counter=0;
 	for( LLSD::array_const_iterator list = mLists.beginArray(), endList = mLists.endArray();
-		 list != endList && (*list).isDefined(); // deleting entries leaves undefined items in the iterator 
+		 list != endList;
 		 list++
 		)
 	{
@@ -377,18 +387,21 @@ LLSD LLAutoReplaceSettings::getListNames()
 			if ( thisList.has(AUTOREPLACE_LIST_NAME) )
 			{
 				std::string name = thisList[AUTOREPLACE_LIST_NAME].asString();
+				LL_DEBUGS("AutoReplace")<<counter<<" '"<<name<<"'"<<LL_ENDL;
 				toReturn.append(LLSD(name));
 			}
 			else
 			{
-				LL_ERRS("AutoReplace") << "  ! MISSING "<<AUTOREPLACE_LIST_NAME<< LL_ENDL;
+				LL_ERRS("AutoReplace") <<counter<<" ! MISSING "<<AUTOREPLACE_LIST_NAME<< LL_ENDL;
 			}
 		}
 		else
 		{
-			LL_ERRS("AutoReplace") << "  ! not a map: "<<LLSD::typeString(thisList.type())<< LL_ENDL;
+			LL_WARNS("AutoReplace")<<counter<<" ! not a map: "<<LLSD::typeString(thisList.type())<< LL_ENDL;
 		}
+		counter++;
 	}
+	LL_DEBUGS("AutoReplace")<<"^^^^^^"<<LL_ENDL;
 	return toReturn;
 }
 
@@ -544,29 +557,42 @@ bool LLAutoReplaceSettings::increaseListPriority(std::string listName)
 {
 	LL_DEBUGS("AutoReplace")<<listName<<LL_ENDL;
 	bool found = false;
-	S32 search_index;
+	S32 search_index, previous_index;
 	LLSD targetList;
+	// The following is working around the fact that LLSD arrays containing maps also seem to have undefined entries... see LLSD-30
+	previous_index = -1;
 	for ( search_index = 0, targetList = mLists[0];
-		  !found && targetList.isDefined();
+		  !found && search_index < mLists.size();
 		  search_index += 1, targetList = mLists[search_index]
 		 )
 	{
-		if ( listNameMatches( targetList, listName) )
+		if ( targetList.isMap() )
 		{
-			LL_DEBUGS("AutoReplace")<<"found at index "<<search_index<<LL_ENDL;
-			found = true;
-			if (search_index > 0)
+			if ( listNameMatches( targetList, listName) )
 			{
-				// copy the target to before the element preceding it
-				mLists.insert(search_index-1,targetList);
-				// delete the original, now duplicate, copy
-				mLists.erase(search_index+1);
+				LL_DEBUGS("AutoReplace")<<"found at "<<search_index<<", previous is "<<previous_index<<LL_ENDL;
+				found = true;
+				if (previous_index >= 0)
+				{
+					LL_DEBUGS("AutoReplace") << "erase "<<search_index<<LL_ENDL;
+					mLists.erase(search_index);
+					LL_DEBUGS("AutoReplace") << "insert at "<<previous_index<<LL_ENDL;
+					mLists.insert(previous_index, targetList);
+				} 
+				else
+				{
+					LL_WARNS("AutoReplace") << "attempted to move top list up" << LL_ENDL;
+				}
 			}
 			else
 			{
-				LL_WARNS("AutoReplace") << "attempted to move top list up" << LL_ENDL;
+				previous_index = search_index;
 			}
 		}
+		else
+		{
+			LL_DEBUGS("AutoReplace") << search_index<<" is "<<LLSD::typeString(targetList.type())<<LL_ENDL;
+		}		
 	}
 	return found;
 }
@@ -575,10 +601,10 @@ bool LLAutoReplaceSettings::increaseListPriority(std::string listName)
 bool LLAutoReplaceSettings::decreaseListPriority(std::string listName)
 {
 	LL_DEBUGS("AutoReplace")<<listName<<LL_ENDL;
-	S32 found_index = -1;
+	S32 found_index = -1;	
 	S32 search_index;
 	for ( search_index = 0;
-		  found_index == -1 && mLists[search_index].isDefined();
+		  found_index == -1 && search_index < mLists.size();
 		  search_index++
 		 )
 	{
@@ -590,19 +616,32 @@ bool LLAutoReplaceSettings::decreaseListPriority(std::string listName)
 	}
 	if (found_index != -1)
 	{
-		// move this list after the found_index from after it to before it
-		if (mLists[found_index+1].isDefined())
+		S32 next_index;
+		for ( next_index = found_index+1;
+			  next_index < mLists.size() && ! mLists[next_index].isMap();
+			  next_index++
+			 )
+		{
+			// skipping over any undefined slots (see LLSD-30)
+			LL_WARNS("AutoReplace")<<next_index<<" ! not a map: "<<LLSD::typeString(mLists[next_index].type())<< LL_ENDL;
+		}
+		if ( next_index < mLists.size() )
 		{
-			// copy item to after the element that follows it
-			mLists.insert(found_index+2, mLists[found_index]);
-			// erase the original, now duplicate, copy
-			mLists.erase(found_index);
+			LLSD next_list = mLists[next_index];
+			LL_DEBUGS("AutoReplace") << "erase "<<next_index<<LL_ENDL;
+			mLists.erase(next_index);
+			LL_DEBUGS("AutoReplace") << "insert at "<<found_index<<LL_ENDL;
+			mLists.insert(found_index, next_list);
 		}
 		else
 		{
 			LL_WARNS("AutoReplace") << "attempted to move bottom list down" << LL_ENDL;
 		}
 	}
+	else
+	{
+		LL_WARNS("AutoReplace") << "not found" << LL_ENDL;
+	}
 	return (found_index != -1);
 }
 
@@ -722,6 +761,11 @@ LLSD LLAutoReplaceSettings::getExampleLLSD()
 	return example;
 }
 
+const LLSD& LLAutoReplaceSettings::getAsLLSD()
+{
+	return mLists;
+}
+
 LLAutoReplaceSettings::~LLAutoReplaceSettings()
 {
 }
diff --git a/indra/newview/llautoreplace.h b/indra/newview/llautoreplace.h
index 944676feddf..c097e96b9e8 100644
--- a/indra/newview/llautoreplace.h
+++ b/indra/newview/llautoreplace.h
@@ -42,7 +42,7 @@ class LLAutoReplaceSettings
 	/// Constructor for creating a tempory copy of the current settings
 	LLAutoReplaceSettings(const LLAutoReplaceSettings& settings);
 
-	/// Replace the current settings with new ones
+	/// Replace the current settings with new ones and save them to the user settings file
 	void set(const LLAutoReplaceSettings& newSettings);
 	
 	/// Load the current settings read from an LLSD file
@@ -127,6 +127,10 @@ class LLAutoReplaceSettings
 
 	/// Provides a hard-coded example of settings 
 	LLSD getExampleLLSD();
+
+	/// Get the actual settings as LLSD
+	const LLSD& getAsLLSD();
+	///< @note for use only in AutoReplace::saveToUserSettings
 	
   private:
 	/// Efficiently and safely compare list names 
@@ -193,9 +197,8 @@ class LLAutoReplace : public LLSingleton<LLAutoReplace>
 	/// Callback that provides the hook for use in text entry methods
 	void autoreplaceCallback(LLUIString& inputText, S32& cursorPos);
 
-	/// Get a copy of the current settings to be manipulated in the preferences dialog
+	/// Get a copy of the current settings
 	LLAutoReplaceSettings getSettings();
-	///< @note: the caller is respon
 
 	/// Commit new settings after making changes
 	void setSettings(const LLAutoReplaceSettings& settings);
@@ -206,12 +209,12 @@ class LLAutoReplace : public LLSingleton<LLAutoReplace>
 
 	LLAutoReplaceSettings mSettings; ///< configuration information
 	
-	/// Write the current lists out to the settings storage
-	void saveToUserSettings(const LLSD& newSettings);
-
 	/// Read settings from persistent storage
 	void loadFromSettings();
 
+	/// Make the newSettings active and write them to user storage
+	void saveToUserSettings();
+
 	/// Compute the user settings file name
 	std::string getUserSettingsFileName();
 
diff --git a/indra/newview/llfloaterautoreplacesettings.cpp b/indra/newview/llfloaterautoreplacesettings.cpp
index 7d6be105496..64b3b092769 100644
--- a/indra/newview/llfloaterautoreplacesettings.cpp
+++ b/indra/newview/llfloaterautoreplacesettings.cpp
@@ -302,7 +302,7 @@ void LLFloaterAutoReplaceSettings::onListUp()
 
 	if ( mSettings.increaseListPriority(selectedName) )
 	{
-		mListNames->swapWithPrevious(selectedRow);
+		updateListNames();
 		updateListNamesControls();
 	}
 	else
@@ -321,7 +321,7 @@ void LLFloaterAutoReplaceSettings::onListDown()
 
 	if ( mSettings.decreaseListPriority(selectedName) )
 	{
-		mListNames->swapWithNext(selectedRow);
+		updateListNames();
 		updateListNamesControls();
 	}
 	else
@@ -529,6 +529,7 @@ void LLFloaterAutoReplaceSettings::onExportList()
 void LLFloaterAutoReplaceSettings::onAddEntry()
 {
 	mPreviousKeyword.clear();
+	mReplacementsList->deselectAllItems(false /* don't call commit */);
 	mKeyword->clear();
 	mReplacement->clear();
 	enableReplacementEntry();
-- 
GitLab