rgheck wrote:
Jean-Marc Lasgouttes wrote:
The cause of the bug is that the counter-reading code overwrites an existing counter when it is read again, rather than updating it, as for other bits of layout information. This patch fixes the bug and, along the way, makes the counter-reading code more like the style-reading code. The bulk of it is moved to Counters.cpp.

I am all for it if it is corectly tested (but of course, this looks more like cleanup than bug fixing...).

Some of it is cleanup, yes, but it's the best way to fix the bug, I think. It's messy if you don't make this kind of change, because we have to start reading the counter to get the Name, and that's not actually required to be the first entry in the Counter declaration. So we can't, say, get the old counter and update it instead of reading a new one.

But the real issue is that we have no way of accessing the original counter anyway. You can of course get to the CounterList, via TextClass::counters_, but there's no Counters::operator[] or anything of the sort. Of course, we could add that---but do we really want it? And there are other options, but they all involve messing with CounterList, too.

I've tested this some, but of course more would be welcome. The layouts still open properly, with layout2layout conversion. The counters still work, with no layout errors. Here's how to see the bug fix: Open the Introduction; change the Document Class to paper; the numbering is currently messed up (0.1, e.g.). With the patch, it should be ok.

Attached is the patch without the extraneous cleanup.

rh

Index: src/Counters.h
===================================================================
--- src/Counters.h	(revision 26778)
+++ src/Counters.h	(working copy)
@@ -23,6 +23,8 @@
 
 namespace lyx {
 
+class Lexer;
+
 /// This represents a single counter.
 class Counter {
 public:
@@ -31,6 +33,8 @@
 	///
 	Counter(docstring const & mc, docstring const & ls, 
 		docstring const & lsa);
+	/// \return true on success
+	bool read(Lexer & lex);
 	///
 	void set(int v);
 	///
@@ -75,8 +79,6 @@
 public:
 	///
 	Counters() : appendix_(false), subfloat_(false) {}
-	/// Add a new counter to array.
-	void newCounter(docstring const & newc);
 	/// Add new counter newc having masterc as its master, 
 	/// ls as its label, and lsa as its appendix label.
 	void newCounter(docstring const & newc,
@@ -85,6 +87,9 @@
 			docstring const & lsa);
 	/// Checks whether the given counter exists.
 	bool hasCounter(docstring const & c) const;
+	/// reads the counter name
+	/// \return true on success
+	bool read(Lexer & lex, docstring const & name);
 	///
 	void set(docstring const & ctr, int val);
 	///
Index: src/Counters.cpp
===================================================================
--- src/Counters.cpp	(revision 26778)
+++ src/Counters.cpp	(working copy)
@@ -15,6 +15,8 @@
 
 #include "Counters.h"
 
+#include "Lexer.h"
+
 #include "support/convert.h"
 #include "support/debug.h"
 #include "support/lstrings.h"
@@ -43,6 +45,63 @@
 }
 
 
+bool Counter::read(Lexer & lex)
+{
+	enum {
+		CT_WITHIN = 1,
+		CT_LABELSTRING,
+		CT_LABELSTRING_APPENDIX,
+		CT_END
+	};
+
+	LexerKeyword counterTags[] = {
+		{ "end", CT_END },
+		{ "labelstring", CT_LABELSTRING },
+		{ "labelstringappendix", CT_LABELSTRING_APPENDIX },
+		{ "within", CT_WITHIN }
+	};
+
+	lex.pushTable(counterTags);
+
+	bool getout = false;
+	while (!getout && lex.isOK()) {
+		int le = lex.lex();
+		switch (le) {
+			case Lexer::LEX_UNDEF:
+				lex.printError("Unknown counter tag `$$Token'");
+				continue;
+			default: 
+				break;
+		}
+		switch (le) {
+			case CT_WITHIN:
+				lex.next();
+				master_ = lex.getDocString();
+				if (master_ == "none")
+					master_.erase();
+				break;
+			case CT_LABELSTRING:
+				lex.next();
+				labelstring_ = lex.getDocString();
+				labelstringappendix_ = labelstring_;
+				break;
+			case CT_LABELSTRING_APPENDIX:
+				lex.next();
+				labelstringappendix_ = lex.getDocString();
+				break;
+			case CT_END:
+				getout = true;
+				break;
+		}
+	}
+
+	// Here if have a full counter if getout == true
+	if (!getout)
+		LYXERR0("No End tag found for counter!");
+	lex.popTable();
+	return getout;
+}
+
 void Counter::set(int v)
 {
 	value_ = v;
@@ -112,6 +171,23 @@
 }
 
 
+bool Counters::read(Lexer & lex, docstring const & name)
+{
+	if (hasCounter(name)) {
+		LYXERR(Debug::TCLASS, "Reading existing counter " << to_utf8(name));
+		return counterList[name].read(lex);
+	}
+	LYXERR(Debug::TCLASS, "Reading new counter " << to_utf8(name));
+	Counter cnt;
+	bool success = cnt.read(lex);
+	if (success)
+		counterList[name] = cnt;
+	else
+		LYXERR0("Error reading counter `" << name << "'!");
+	return success;
+}
+
+
 void Counters::set(docstring const & ctr, int const val)
 {
 	CounterList::iterator const it = counterList.find(ctr);
Index: src/TextClass.h
===================================================================
--- src/TextClass.h	(revision 26778)
+++ src/TextClass.h	(working copy)
@@ -305,8 +305,6 @@
 	void readCharStyle(Lexer &, std::string const &);
 	///
 	void readFloat(Lexer &);
-	///
-	void readCounter(Lexer &);
 };
 
 
Index: src/TextClass.cpp
===================================================================
--- src/TextClass.cpp	(revision 26778)
+++ src/TextClass.cpp	(working copy)
@@ -61,7 +61,7 @@
 };
 
 
-int const FORMAT = 9;
+int const FORMAT = 10;
 
 
 bool layout2layout(FileName const & filename, FileName const & tempfile)
@@ -520,7 +520,23 @@
 			break;
 
 		case TC_COUNTER:
-			readCounter(lexrc);
+			if (lexrc.next()) {
+				docstring const name = lexrc.getDocString();
+				if (name.empty()) {
+					string s = "Could not read name for counter: `$$Token' "
+							+ lexrc.getString() + " is probably not valid UTF-8!";
+					lexrc.printError(s.c_str());
+					Counter c;
+					// Since we couldn't read the name, we just scan the rest
+					// and discard it.
+					c.read(lexrc);
+				} else
+					error = !counters_.read(lexrc, name);
+			}
+			else {
+				lexrc.printError("No name given for style: `$$Token'.");
+				error = true;
+			}
 			break;
 
 		case TC_TITLELATEXTYPE:
@@ -825,79 +841,6 @@
 }
 
 
-void TextClass::readCounter(Lexer & lexrc)
-{
-	enum {
-		CT_NAME = 1,
-		CT_WITHIN,
-		CT_LABELSTRING,
-		CT_LABELSTRING_APPENDIX,
-		CT_END
-	};
-
-	LexerKeyword counterTags[] = {
-		{ "end", CT_END },
-		{ "labelstring", CT_LABELSTRING },
-		{ "labelstringappendix", CT_LABELSTRING_APPENDIX },
-		{ "name", CT_NAME },
-		{ "within", CT_WITHIN }
-	};
-
-	lexrc.pushTable(counterTags);
-
-	docstring name;
-	docstring within;
-	docstring labelstring;
-	docstring labelstring_appendix;
-
-	bool getout = false;
-	while (!getout && lexrc.isOK()) {
-		int le = lexrc.lex();
-		switch (le) {
-		case Lexer::LEX_UNDEF:
-			lexrc.printError("Unknown counter tag `$$Token'");
-			continue;
-		default: break;
-		}
-		switch (le) {
-		case CT_NAME:
-			lexrc.next();
-			name = lexrc.getDocString();
-			if (counters_.hasCounter(name))
-				LYXERR(Debug::TCLASS, "Reading existing counter " << to_utf8(name));
-			else
-				LYXERR(Debug::TCLASS, "Reading new counter " << to_utf8(name));
-			break;
-		case CT_WITHIN:
-			lexrc.next();
-			within = lexrc.getDocString();
-			if (within == "none")
-				within.erase();
-			break;
-		case CT_LABELSTRING:
-			lexrc.next();
-			labelstring = lexrc.getDocString();
-			labelstring_appendix = labelstring;
-			break;
-		case CT_LABELSTRING_APPENDIX:
-			lexrc.next();
-			labelstring_appendix = lexrc.getDocString();
-			break;
-		case CT_END:
-			getout = true;
-			break;
-		}
-	}
-
-	// Here if have a full counter if getout == true
-	if (getout)
-		counters_.newCounter(name, within, 
-				      labelstring, labelstring_appendix);
-
-	lexrc.popTable();
-}
-
-
 bool TextClass::hasLayout(docstring const & n) const
 {
 	docstring const name = n.empty() ? defaultLayoutName() : n;
Index: lib/scripts/layout2layout.py
===================================================================
--- lib/scripts/layout2layout.py	(revision 26778)
+++ lib/scripts/layout2layout.py	(working copy)
@@ -33,9 +33,12 @@
 # Incremented to format 9, 5 October 2008 by rgh
 # ForcePlain and CustomPars tags added to InsetLayout
 
-currentFormat = 9
+# Incremented to format 10, 6 October 2008 by rgh
+# Change format of counters
 
+currentFormat = 10
 
+
 def usage(prog_name):
     return ("Usage: %s inputfile outputfile\n" % prog_name +
             "or     %s <inputfile >outputfile" % prog_name)
@@ -87,6 +90,8 @@
 def convert(lines):
     " Convert to new format."
     re_Comment = re.compile(r'^(\s*)#')
+    re_Counter = re.compile(r'\s*Counter\s*')
+    re_Name = re.compile(r'\s*Name\s+(\S+)\s*')
     re_Empty = re.compile(r'^(\s*)$')
     re_Format = re.compile(r'^(\s*)(Format)(\s+)(\S+)', re.IGNORECASE)
     re_Preamble = re.compile(r'^(\s*)Preamble', re.IGNORECASE)
@@ -175,6 +180,27 @@
                 i += 1
             continue
 
+        if format == 9:
+            match = re_Counter.match(lines[i])
+            if match:
+                counterline = i
+                i += 1
+                while i < len(lines):
+                    namem = re_Name.match(lines[i])
+                    if namem:
+                        name = namem.group(1)
+                        lines.pop(i)
+                        lines[counterline] = "Counter %s" % name
+                        # we don't need to increment i
+                        continue
+                    endem = re_End.match(lines[i])
+                    if endem:
+                        i += 1
+                        break
+                    i += 1
+            i += 1
+            continue
+
         # These just involved new features, not any changes to old ones
         if format >= 5 and format <= 8:
           i += 1

Reply via email to