Modify logging so that the in-viewer console and stderr do not escape line breaks

Improve the implementation so that escaping is computed only once

8 files changed, 152 insertions, 107 deletions

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 6dfb4bf028..668ea1f7d2 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -119,8 +119,6 @@ namespace {
 			{
 				LL_INFOS() << "Error setting log file to " << filename << LL_ENDL;
 			}
-			mWantsTime = true;
-            mWantsTags = true;
 		}
 		
 		~RecordToFile()
@@ -146,7 +144,7 @@ namespace {
 	public:
 		RecordToStderr(bool timestamp) : mUseANSI(ANSI_PROBE) 
 		{
-			mWantsTime = timestamp;
+            this->showMultiline(true);
 		}
 		
 		virtual void recordMessage(LLError::ELevel level,
@@ -207,7 +205,13 @@ namespace {
 	class RecordToFixedBuffer : public LLError::Recorder
 	{
 	public:
-		RecordToFixedBuffer(LLLineBuffer* buffer) : mBuffer(buffer) { }
+		RecordToFixedBuffer(LLLineBuffer* buffer)
+            : mBuffer(buffer)
+            {
+                this->showMultiline(true);
+                this->showTags(false);
+                this->showLocation(false);
+            }
 		
 		virtual void recordMessage(LLError::ELevel level,
 								   const std::string& message)
@@ -224,7 +228,11 @@ namespace {
 	{
 	public:
 		RecordToWinDebug()
-		{}
+		{
+            this->showMultiline(true);
+            this->showTags(false);
+            this->showLocation(false);
+        }
 
 		virtual void recordMessage(LLError::ELevel level,
 								   const std::string& message)
@@ -411,8 +419,6 @@ namespace LLError
 	public:
 		virtual ~SettingsConfig();
 
-		bool                                mPrintLocation;
-
 		LLError::ELevel                     mDefaultLevel;
 		
 		LevelMap                            mFunctionLevelMap;
@@ -453,7 +459,6 @@ namespace LLError
 
 	SettingsConfig::SettingsConfig()
 		: LLRefCount(),
-		mPrintLocation(false),
 		mDefaultLevel(LLError::LEVEL_DEBUG),
 		mFunctionLevelMap(),
 		mClassLevelMap(),
@@ -655,12 +660,6 @@ namespace LLError
 		commonInit(user_dir, app_dir, log_to_stderr);
 	}
 
-	void setPrintLocation(bool print)
-	{
-		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
-		s->mPrintLocation = print;
-	}
-
 	void setFatalFunction(const FatalFunction& f)
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
@@ -775,7 +774,6 @@ namespace LLError
 		s->mTagLevelMap.clear();
 		s->mUniqueLogMessages.clear();
 		
-		setPrintLocation(config["print-location"]);
 		setDefaultLevel(decodeLevel(config["default-level"]));
 		
 		LLSD sets = config["settings"];
@@ -798,11 +796,12 @@ namespace LLError
 namespace LLError
 {
 	Recorder::Recorder()
-	:	mWantsTime(false),
-		mWantsTags(false),
-		mWantsLevel(true),
-		mWantsLocation(false),
-		mWantsFunctionName(true)
+    	: mWantsTime(true)
+        , mWantsTags(true)
+        , mWantsLevel(true)
+        , mWantsLocation(true)
+        , mWantsFunctionName(true)
+        , mWantsMultiline(false)
 	{
 	}
 
@@ -839,6 +838,42 @@ namespace LLError
 		return mWantsFunctionName;
 	}
 
+	// virtual 
+	bool Recorder::wantsMultiline() 
+	{ 
+		return mWantsMultiline;
+	}
+
+    void Recorder::showTime(bool show)
+    {
+        mWantsTime = show;
+    }
+    
+    void Recorder::showTags(bool show)
+    {
+        mWantsTags = show;
+    }
+
+    void Recorder::showLevel(bool show)
+    {
+        mWantsLevel = show;
+    }
+
+    void Recorder::showLocation(bool show)
+    {
+        mWantsLocation = show;
+    }
+
+    void Recorder::showFunctionName(bool show)
+    {
+        mWantsFunctionName = show;
+    }
+
+    void Recorder::showMultiline(bool show)
+    {
+        mWantsMultiline = show;
+    }
+
 	void addRecorder(RecorderPtr recorder)
 	{
 		if (!recorder)
@@ -871,17 +906,15 @@ namespace LLError
 		s->mFileRecorder.reset();
 		s->mFileRecorderFileName.clear();
 		
-		if (file_name.empty())
-		{
-			return;
-		}
-		
-		RecorderPtr recordToFile(new RecordToFile(file_name));
-		if (boost::dynamic_pointer_cast<RecordToFile>(recordToFile)->okay())
+		if (!file_name.empty())
 		{
-			s->mFileRecorderFileName = file_name;
-			s->mFileRecorder = recordToFile;
-			addRecorder(recordToFile);
+            RecorderPtr recordToFile(new RecordToFile(file_name));
+            if (boost::dynamic_pointer_cast<RecordToFile>(recordToFile)->okay())
+            {
+                s->mFileRecorderFileName = file_name;
+                s->mFileRecorder = recordToFile;
+                addRecorder(recordToFile);
+            }
 		}
 	}
 	
@@ -892,14 +925,12 @@ namespace LLError
 		removeRecorder(s->mFixedBufferRecorder);
 		s->mFixedBufferRecorder.reset();
 		
-		if (!fixedBuffer)
+		if (fixedBuffer)
 		{
-			return;
-		}
-		
-		RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
-		s->mFixedBufferRecorder = recordToFixedBuffer;
-		addRecorder(recordToFixedBuffer);
+            RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
+            s->mFixedBufferRecorder = recordToFixedBuffer;
+            addRecorder(recordToFixedBuffer);
+        }
 	}
 
 	std::string logFileName()
@@ -911,8 +942,9 @@ namespace LLError
 
 namespace
 {
-    void addEscapedMessage(std::ostream& out, const std::string& message)
+    std::string escapedMessageLines(const std::string& message)
     {
+        std::ostringstream out;
         size_t written_out = 0;
         size_t all_content = message.length();
         size_t escape_char_index; // always relative to start of message
@@ -948,13 +980,16 @@ namespace
             // write whatever was left
             out << message.substr(written_out, std::string::npos);
         }
+        return out.str();
     }
 
-	void writeToRecorders(const LLError::CallSite& site, const std::string& escaped_message, bool show_location = true, bool show_time = true, bool show_tags = true, bool show_level = true, bool show_function = true)
+	void writeToRecorders(const LLError::CallSite& site, const std::string& message)
 	{
 		LLError::ELevel level = site.mLevel;
 		LLError::SettingsConfigPtr s = LLError::Settings::getInstance()->getSettingsConfig();
-	
+
+        std::string escaped_message;
+        
 		for (Recorders::const_iterator i = s->mRecorders.begin();
 			i != s->mRecorders.end();
 			++i)
@@ -969,7 +1004,7 @@ namespace
 			}
             message_stream << " ";
             
-			if (show_level && r->wantsLevel())
+			if (r->wantsLevel())
             {
 				message_stream << site.mLevelString;
             }
@@ -981,19 +1016,30 @@ namespace
 			}
             message_stream << " ";
 
-            if (r->wantsLocation() || level == LLError::LEVEL_ERROR || s->mPrintLocation)
+            if (r->wantsLocation() || level == LLError::LEVEL_ERROR)
             {
                 message_stream << site.mLocationString;
             }
             message_stream << " ";
 
-			if (show_function && r->wantsFunctionName())
+			if (r->wantsFunctionName())
 			{
 				message_stream << site.mFunctionString;
 			}
             message_stream << " : ";
 
-			message_stream << escaped_message;
+            if (r->wantsMultiline())
+            {
+                message_stream << message;
+            }
+            else
+            {
+                if (escaped_message.empty())
+                {
+                    escaped_message = escapedMessageLines(message);
+                }
+                message_stream << escaped_message;
+            }
 
 			r->recordMessage(level, message_stream.str());
 		}
@@ -1236,10 +1282,11 @@ namespace LLError
 			delete out;
 		}
 
-		std::ostringstream message_stream;
 
 		if (site.mPrintOnce)
 		{
+            std::ostringstream message_stream;
+
 			std::map<std::string, unsigned int>::iterator messageIter = s->mUniqueLogMessages.find(message);
 			if (messageIter != s->mUniqueLogMessages.end())
 			{
@@ -1259,19 +1306,18 @@ namespace LLError
 				message_stream << "ONCE: ";
 				s->mUniqueLogMessages[message] = 1;
 			}
+            message_stream << message;
+            message = message_stream.str();
 		}
 		
-		addEscapedMessage(message_stream, message);
-		std::string message_line(message_stream.str());
-
-		writeToRecorders(site, message_line);
+		writeToRecorders(site, message);
 
 		if (site.mLevel == LEVEL_ERROR)
 		{
-			g->mFatalMessage = message_line;
+			g->mFatalMessage = message;
 			if (s->mCrashFunction)
 			{
-				s->mCrashFunction(message_line);
+				s->mCrashFunction(message);
 			}
 		}
 	}
@@ -1579,3 +1625,4 @@ bool debugLoggingEnabled(const std::string& tag)
 }
 
 
+
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index ddbcdc94a0..a6278b3e50 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -148,13 +148,22 @@ namespace LLError
 		bool wantsLevel();
 		bool wantsLocation(); 
 		bool wantsFunctionName();
+        bool wantsMultiline();
+
+		void showTime(bool show);
+		void showTags(bool show);
+		void showLevel(bool show);
+		void showLocation(bool show); 
+		void showFunctionName(bool show);
+		void showMultiline(bool show);
 
 	protected:
-		bool	mWantsTime,
-				mWantsTags,
-				mWantsLevel,
-				mWantsLocation,
-				mWantsFunctionName;
+		bool mWantsTime;
+        bool mWantsTags;
+        bool mWantsLevel;
+        bool mWantsLocation;
+        bool mWantsFunctionName;
+        bool mWantsMultiline;
 	};
 
 	typedef boost::shared_ptr<Recorder> RecorderPtr;
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index ce0dbce075..bd0357e4bf 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -78,8 +78,12 @@ namespace tut
 	class TestRecorder : public LLError::Recorder
 	{
 	public:
-		TestRecorder() { mWantsTime = false; mWantsTags = true; }
-		virtual ~TestRecorder() {  }
+		TestRecorder()
+            {
+                showTime(false);
+            }
+		virtual ~TestRecorder()
+            {}
 
 		virtual void recordMessage(LLError::ELevel level,
 						   const std::string& message)
@@ -90,8 +94,6 @@ namespace tut
 		int countMessages()			{ return (int) mMessages.size(); }
 		void clearMessages()		{ mMessages.clear(); }
 
-		void setWantsTime(bool t)	{ mWantsTime = t; }
-
 		std::string message(int n)
 		{
 			std::ostringstream test_name;
@@ -139,9 +141,14 @@ namespace tut
 		}
 
 		void setWantsTime(bool t)
-		{
-			boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->setWantsTime(t);
-		}
+            {
+                boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->showTime(t);
+            }
+
+		void setWantsMultiline(bool t)
+            {
+                boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->showMultiline(t);
+            }
 
 		std::string message(int n)
 		{
@@ -378,27 +385,6 @@ namespace
 	}
 }
 
-namespace tut
-{
-	template<> template<>
-	void ErrorTestObject::test<5>()
-		// file and line information in log messages
-	{
-		std::string location = writeReturningLocation();
-			// expecting default to not print location information
-
-		LLError::setPrintLocation(true);
-		writeReturningLocation();
-
-		LLError::setPrintLocation(false);
-		writeReturningLocation();
-
-		ensure_message_does_not_contain(0, location);
-		ensure_message_field_equals(1, LOCATION_FIELD, location);
-		ensure_message_does_not_contain(2, location);
-	}
-}
-
 /* The following helper functions and class members all log a simple message
 	from some particular function scope.  Each function takes a bool argument
 	that indicates if it should log its own name or not (in the manner that
@@ -583,7 +569,6 @@ namespace tut
 		// special handling of LL_ERRS() calls
 	void ErrorTestObject::test<8>()
 	{
-		LLError::setPrintLocation(false);
 		std::string location = errorReturningLocation();
 
 		ensure_message_field_equals(0, LOCATION_FIELD, location);
@@ -630,15 +615,15 @@ namespace tut
 		// output order
 	void ErrorTestObject::test<10>()
 	{
-		LLError::setPrintLocation(true);
 		LLError::setTimeFunction(roswell);
 		setWantsTime(true);
+
 		std::string location,
 					function;
 		writeReturningLocationAndFunction(location, function);
 
 		ensure_equals("order is time level tags location function message",
-			message(0),
+                      message(0),
                       roswell() + " INFO " + "# " /* no tag */ + location + " " + function + " : " + "apple");
 	}
 
@@ -658,7 +643,7 @@ namespace tut
 		LLError::setTimeFunction(roswell);
 
 		LLError::RecorderPtr anotherRecorder(new TestRecorder());
-		boost::dynamic_pointer_cast<TestRecorder>(anotherRecorder)->setWantsTime(true);
+		boost::dynamic_pointer_cast<TestRecorder>(anotherRecorder)->showTime(true);
 		LLError::addRecorder(anotherRecorder);
 
 		LL_INFOS() << "baz" << LL_ENDL;
@@ -896,6 +881,25 @@ namespace tut
     }
 }
 
+namespace tut
+{
+    template<> template<>
+    void ErrorTestObject::test<19>()
+        // backslash, return, and newline are not escaped with backslashes
+    {
+        LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
+        setWantsMultiline(true); 
+        writeMsgNeedsEscaping(); // but should not be now
+        ensure_message_field_equals(0, MSG_FIELD, "backslash\\");
+        ensure_message_field_equals(1, MSG_FIELD, "newline\nafternewline");
+        ensure_message_field_equals(2, MSG_FIELD, "return\rafterreturn");
+        ensure_message_field_equals(3, MSG_FIELD, "backslash\\backslash\\");
+        ensure_message_field_equals(4, MSG_FIELD, "backslash\\newline\nanothernewline\nafternewline");
+        ensure_message_field_equals(5, MSG_FIELD, "backslash\\returnnewline\r\n\\afterbackslash");
+        ensure_message_count(6);
+    }
+}
+
 /* Tests left:
 	handling of classes without LOG_CLASS
 
diff --git a/indra/newview/app_settings/logcontrol.xml b/indra/newview/app_settings/logcontrol.xml
index 71445ea541..8ced81fdb3 100644
--- a/indra/newview/app_settings/logcontrol.xml
+++ b/indra/newview/app_settings/logcontrol.xml
@@ -2,7 +2,6 @@
 	<map>
 		<!-- default-level can be ALL, DEBUG, INFO, WARN, ERROR, or NONE -->
 		<key>default-level</key>    <string>INFO</string>
-		<key>print-location</key>   <boolean>true</boolean>
 		<key>settings</key>
 			<array>
 				<!-- Suppress anything but ERROR for some very verbose components -->
diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml
index 8e8cce5787..44aa22b387 100644
--- a/indra/newview/app_settings/settings.xml
+++ b/indra/newview/app_settings/settings.xml
@@ -13980,17 +13980,6 @@
       <key>Value</key>
       <integer>1</integer>
     </map>
-    <key>VerboseLogs</key>
-    <map>
-      <key>Comment</key>
-      <string>Display source file and line number for each log item for debugging purposes</string>
-      <key>Persist</key>
-      <integer>1</integer>
-      <key>Type</key>
-      <string>Boolean</string>
-      <key>Value</key>
-      <integer>0</integer>
-    </map>
     <key>VertexShaderEnable</key>
     <map>
       <key>Comment</key>
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index dd82aa735f..b3f09a73a2 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -885,11 +885,6 @@ bool LLAppViewer::init()
 	mNumSessions++;
 	gSavedSettings.setS32("NumSessions", mNumSessions);
 
-	if (gSavedSettings.getBOOL("VerboseLogs"))
-	{
-		LLError::setPrintLocation(true);
-	}
-
 	// LLKeyboard relies on LLUI to know what some accelerator keys are called.
 	LLKeyboard::setStringTranslatorFunc( LLTrans::getKeyboardString );
 
@@ -1702,7 +1697,7 @@ bool LLAppViewer::cleanup()
 
 	release_start_screen(); // just in case
 
-	LLError::logToFixedBuffer(NULL);
+	LLError::logToFixedBuffer(NULL); // stop the fixed buffer recorder
 
 	LL_INFOS() << "Cleaning Up" << LL_ENDL;
 
diff --git a/indra/newview/llviewerwindow.cpp b/indra/newview/llviewerwindow.cpp
index 01ec703fe6..873d011acc 100644
--- a/indra/newview/llviewerwindow.cpp
+++ b/indra/newview/llviewerwindow.cpp
@@ -306,6 +306,9 @@ private:
 RecordToChatConsole::RecordToChatConsole():
 	mRecorder(new RecordToChatConsoleRecorder())
 {
+    mRecorder->showTags(false);
+    mRecorder->showLocation(false);
+    mRecorder->showMultiline(true);
 }
 
 ////////////////////////////////////////////////////////////////////////////
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index 80c84d9bea..9d6e9539cb 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -535,7 +535,6 @@ int main(int argc, char **argv)
 		LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
 	}	
 	LLError::setFatalFunction(wouldHaveCrashed);
-	LLError::setPrintLocation(true);
 	std::string test_app_name(argv[0]);
 	std::string test_log = test_app_name + ".log";
 	LLFile::remove(test_log);