From: Angus Leeming <leeming@lyx.org>
Date: Sun, 14 Dec 2003 16:33:56 +0000 (+0000)
Subject: Overhaul the branches code.
X-Git-Tag: 1.6.10~15648
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=1cad117882dc393b2d55174c6f2ab67920a54ce1;p=features.git

Overhaul the branches code.


git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@8247 a592a061-630c-0410-9148-cb99ea01b6c8
---

diff --git a/src/BranchList.C b/src/BranchList.C
index dd9f8871a9..93f117a20e 100644
--- a/src/BranchList.C
+++ b/src/BranchList.C
@@ -22,7 +22,7 @@ using std::bind2nd;
 using std::binary_function;
 
 
-string const Branch::getBranch() const
+string const & Branch::getBranch() const
 {
 	return branch_;
 }
@@ -40,13 +40,16 @@ bool Branch::getSelected() const
 }
 
 
-void Branch::setSelected(bool b)
+bool Branch::setSelected(bool b)
 {
+	if (b == selected_)
+		return false;
 	selected_ = b;
+	return true;
 }
 
 
-string const Branch::getColor() const
+string const & Branch::getColor() const
 {
 	return color_;
 }
@@ -58,60 +61,41 @@ void Branch::setColor(string const & c)
 }
 
 
-void BranchList::clear()
-{
-	list.clear();
-}
-
+namespace {
 
-string BranchList::getColor(string const & s) const
-{
-	List::const_iterator it = list.begin();
-	List::const_iterator end = list.end();
-	for (; it != end; ++it) {
-		if (s == it->getBranch()) {
-			return it->getColor();
-		}
-	}
-	BOOST_ASSERT(false); // Always
-	return string(); // never gets here
-}
+struct SameName {
+	SameName(string const & name) : name_(name) {}
+	bool operator()(Branch const & branch) const
+		{ return branch.getBranch() == name_; }
+private:
+	string name_;
+};
 
+}// namespace anon
 
 
-void BranchList::setColor(string const & s, string const & val)
+Branch * BranchList::find(std::string const & name)
 {
-	List::iterator it = list.begin();
-	List::iterator end = list.end();
-	for (; it != end; ++it) {
-		if (s == it->getBranch()) {
-			it->setColor(val);
-			return;
-		}
-	}
-	BOOST_ASSERT(false);
+	List::iterator it =
+		std::find_if(list.begin(), list.end(), SameName(name));
+	return it == list.end() ? 0 : &*it;
 }
 
-
-void BranchList::setSelected(string const & s, bool val)
+	
+Branch const * BranchList::find(std::string const & name) const
 {
-	List::iterator it = list.begin();
-	List::iterator end = list.end();
-	for (; it != end; ++it) {
-		if (s == it->getBranch()) {
-			it->setSelected(val);
-			return;
-		}
-	}
-	BOOST_ASSERT(false);
+	List::const_iterator it =
+		std::find_if(list.begin(), list.end(), SameName(name));
+	return it == list.end() ? 0 : &*it;
 }
 
-
-void BranchList::add(string const & s)
+	
+bool BranchList::add(string const & s)
 {
+	bool added = false;
 	string::size_type i = 0;
 	while (true) {
-		string::size_type const j = s.find_first_of(separator(), i);
+		string::size_type const j = s.find_first_of(separator_, i);
 		string name;
 		if (j == string::npos)
 			name = s.substr(i);
@@ -128,6 +112,7 @@ void BranchList::add(string const & s)
 			}
 		}
 		if (!already) {
+			added = true;
 			Branch br;
 			br.setBranch(name);
 			br.setSelected(false);
@@ -138,6 +123,7 @@ void BranchList::add(string const & s)
 			break;
 		i = j + 1;
 	}
+	return added;
 }
 
 
@@ -152,58 +138,9 @@ struct match : public binary_function<Branch, string, bool> {
 } // namespace anon.
 
 
-void BranchList::remove(string const & s)
+bool BranchList::remove(string const & s)
 {
+	List::size_type const size = list.size();
 	list.remove_if(bind2nd(match(), s));
-}
-
-
-bool BranchList::selected(string const & s) const
-{
-	List::const_iterator it = list.begin();
-	List::const_iterator end = list.end();
-	for (; it != end; ++it) {
-		if (s == it->getBranch())
-			return it->getSelected();
-	}
-	return false;
-}
-
-
-string BranchList::allBranches() const
-{
-	List::const_iterator it = list.begin();
-	List::const_iterator end = list.end();
-	string ret;
-	for (; it != end; ++it) {
-		ret += it->getBranch() + separator();
-	}
-	// remove final '|':
-	string::size_type i = ret.find_last_of(separator());
-	if (i != string::npos)
-		ret.erase(i);
-	return ret;
-}
-
-
-string BranchList::allSelected() const
-{
-	List::const_iterator it = list.begin();
-	List::const_iterator end = list.end();
-	string ret;
-	for (; it != end; ++it) {
-		if (it->getSelected())
-			ret += it->getBranch() + separator();
-	}
-	// remove final '|':
-	string::size_type i = ret.find_last_of(separator());
-	if (i != string::npos)
-		ret.erase(i);
-	return ret;
-}
-
-
-string const BranchList::separator() const
-{
-	return separator_;
+	return size != list.size();
 }
diff --git a/src/BranchList.h b/src/BranchList.h
index aaa5f15ecd..893d8f9b63 100644
--- a/src/BranchList.h
+++ b/src/BranchList.h
@@ -37,15 +37,17 @@
 class Branch {
 public:
 	///
-	std::string const getBranch() const;
+	std::string const & getBranch() const;
 	///
 	void setBranch(std::string const &);
 	///
 	bool getSelected() const;
+	/** Select/deselect the branch.
+	 *  \return true if the selection status changes.
+	 */
+	bool setSelected(bool);
 	///
-	void setSelected(bool);
-	///
-	std::string const getColor() const;
+	std::string const & getColor() const;
 	///
 	void setColor(std::string const &);
 
@@ -61,41 +63,33 @@ private:
 
 
 class BranchList {
-public:
-	///
-	BranchList() : separator_("|") {}
-
 	///
 	typedef std::list<Branch> List;
+public:
+	typedef List::const_iterator const_iterator;
 
 	///
-	void clear();
+	BranchList() : separator_("|") {}
+
 	///
 	bool empty() { return list.empty(); }
 	///
-	bool size() const { return list.size(); }
-	///
-	List::const_iterator begin() const { return list.begin(); }
-	///
-	List::const_iterator end() const { return list.end(); }
-	///
-	std::string getColor(std::string const &) const;
-	///
-	void setColor(std::string const &, std::string const &);
-	/// Select/deselect multiple branches given in '|'-separated string
-	void setSelected(std::string const &, bool);
-	/// Add multiple branches to list
-	void add(std::string const &);
-	/// remove a branch from list by name
-	void remove(std::string const &);
-	/// return whether this branch is selected
-	bool selected(std::string const &) const;
-	/// return, as a '|'-separated string, all branch names
-	std::string allBranches() const;
-	///
-	std::string allSelected() const;
-	///
-	std::string const separator() const;
+	const_iterator begin() const { return list.begin(); }
+	const_iterator end() const { return list.end(); }
+
+	/** \returns the Branch with \c name. If not found, returns 0.
+	 */
+	Branch * find(std::string const & name);
+	Branch const * find(std::string const & name) const;
+
+	/** Add (possibly multiple (separated by separator())) branches to list
+	 *  \returns true if a branch is added.
+	 */
+	bool add(std::string const &);
+	/** remove a branch from list by name
+	 *  \returns true if a branch is removed.
+	 */
+	bool remove(std::string const &);
 
 private:
 	///
diff --git a/src/ChangeLog b/src/ChangeLog
index 99346b882d..9052b6a238 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,32 @@
+2003-12-14  Angus Leeming  <leeming@lyx.org>
+
+	BranchList.[Ch]: minimize the API.
+	(Branch::getBranch, getColor): now return a 'const &'.
+	(Branch::setSelected) now returns a bool set to true if the
+	selection status changes.
+	(BranchList::clear, size, getColor, setColor, setSelected,
+	allBranches, allSelected, separator): removed.
+	(BranchList::find): new functions, returning the Branch with
+	the given name.
+	(BranchList::add, remove): return a bool indicating that
+	the operation was successful.
+
+	* InsetList.C (insetsOpenCloseBranch): much simplified thanks to a
+	new InsetBranch::isBranchSlected member function.
+
+	* LColor.[Ch]: mimimize the API.
+	(fill): renamed as addColor and made private.
+	(setColor, getGUIName, getX11Name, getLaTeXName): the overloaded
+	versions of these functions taking a string arg have been removed.
+
+	* bufferparams.C (readToken):
+	* lyxfunc.C (dispatch):
+	* lyxrc.C (read): changes due to the altered BranchList and
+	LColor APIs.
+
+	* factory.C (createInset, readInset): changes due to altered
+	InsetBranch c-tor.
+
 2003-12-14  Jürgen Spitzmüller  <j.spitzmueller@gmx.de>
 
 	* factory.C: 
diff --git a/src/InsetList.C b/src/InsetList.C
index 43027b3b91..df45a985db 100644
--- a/src/InsetList.C
+++ b/src/InsetList.C
@@ -15,6 +15,7 @@
 
 #include "buffer.h"
 #include "bufferparams.h"
+#include "BranchList.h"
 #include "BufferView.h"
 #include "debug.h"
 
@@ -139,13 +140,15 @@ void InsetList::insetsOpenCloseBranch(Buffer const & buf)
 	List::iterator it = list.begin();
 	List::iterator end = list.end();
 	for (; it != end; ++it) {
-		if (it->inset && it->inset->lyxCode() == InsetOld::BRANCH_CODE) {
-			InsetBranch * inset = static_cast<InsetBranch *>(it->inset);
-			if (buf.params().branchlist().selected(inset->params().branch)) {
-				inset->open();
-			} else {
-				inset->close();
-			}
+		if (!it->inset ||
+		    it->inset->lyxCode() != InsetOld::BRANCH_CODE)
+			continue;
+
+		InsetBranch * inset = static_cast<InsetBranch *>(it->inset);
+		if (inset->isBranchSelected(buf.params().branchlist())) {
+			inset->open();
+		} else {
+			inset->close();
 		}
 	}
 }
diff --git a/src/LColor.C b/src/LColor.C
index ba42c4b761..96e11cad0d 100644
--- a/src/LColor.C
+++ b/src/LColor.C
@@ -41,12 +41,6 @@ struct ColorEntry {
 
 }
 
-struct TransformEntry {
-	char const * lyxname;
-	int ncolor;
-};
-
-
 struct LColor::Pimpl {
 	///
 	struct information {
@@ -172,152 +166,92 @@ void LColor::operator=(LColor const & c)
 }
 
 
-void LColor::fill(LColor::color c,
-				string const & lyxname,
-				string const & x11name,
-				string const & latexname,
-				string const & guiname)
-{
-	ColorEntry ce;
-	ce.lcolor = c;
-	ce.guiname = guiname.c_str();
-	ce.latexname = latexname.c_str();
-	ce.x11name = x11name.c_str();
-	ce.lyxname = lyxname.c_str();
-	pimpl_->fill(ce);
-}
-
-
 string const LColor::getGUIName(LColor::color c) const
 {
-	Pimpl::InfoTab::const_iterator ici = pimpl_->infotab.find(c);
-	if (ici != pimpl_->infotab.end())
-		return _(ici->second.guiname);
-	return "none";
-}
-
-
-string const LColor::getGUIName(string const &  s) const
-{
-	Pimpl::Transform::const_iterator ici = pimpl_->transform.find(s);
-	if (ici != pimpl_->transform.end()) {
-		Pimpl::InfoTab::const_iterator
-			it = pimpl_->infotab.find(ici->second);
-		if (it != pimpl_->infotab.end())
-			return it->second.guiname;
-	}
+	Pimpl::InfoTab::const_iterator it = pimpl_->infotab.find(c);
+	if (it != pimpl_->infotab.end())
+		return _(it->second.guiname);
 	return "none";
 }
 
 
 string const LColor::getX11Name(LColor::color c) const
 {
-	Pimpl::InfoTab::const_iterator ici = pimpl_->infotab.find(c);
-	if (ici != pimpl_->infotab.end())
-		return ici->second.x11name;
+	Pimpl::InfoTab::const_iterator it = pimpl_->infotab.find(c);
+	if (it != pimpl_->infotab.end())
+		return it->second.x11name;
 
 	lyxerr << "LyX internal error: Missing color"
-		" entry in LColor.C for " << int(c) << endl;
-	lyxerr << "Using black." << endl;
-	return "black";
-}
-
-
-string const LColor::getX11Name(string const & s) const
-{
-	Pimpl::Transform::const_iterator ici = pimpl_->transform.find(s);
-	if (ici != pimpl_->transform.end()) {
-		Pimpl::InfoTab::const_iterator
-			it = pimpl_->infotab.find(ici->second);
-		if (it != pimpl_->infotab.end())
-			return it->second.x11name;
-	}
-	lyxerr << "LyX internal error: Missing color"
-		" entry in LColor.C for " << s << endl;
-	lyxerr << "Using black." << endl;
+		" entry in LColor.C for " << int(c) << '\n'
+	       << "Using black." << endl;
 	return "black";
 }
 
 
 string const LColor::getLaTeXName(LColor::color c) const
 {
-	Pimpl::InfoTab::const_iterator ici = pimpl_->infotab.find(c);
-	if (ici != pimpl_->infotab.end())
-		return ici->second.latexname;
-	return "black";
-}
-
-
-string const LColor::getLaTeXName(string const & s) const
-{
-	Pimpl::Transform::const_iterator ici = pimpl_->transform.find(s);
-	if (ici != pimpl_->transform.end()) {
-		Pimpl::InfoTab::const_iterator
-			it = pimpl_->infotab.find(ici->second);
-		if (it != pimpl_->infotab.end())
-			return it->second.latexname;
-	}
+	Pimpl::InfoTab::const_iterator it = pimpl_->infotab.find(c);
+	if (it != pimpl_->infotab.end())
+		return it->second.latexname;
 	return "black";
 }
 
 
 string const LColor::getLyXName(LColor::color c) const
 {
-	Pimpl::InfoTab::const_iterator ici = pimpl_->infotab.find(c);
-	if (ici != pimpl_->infotab.end())
-		return ici->second.lyxname;
+	Pimpl::InfoTab::const_iterator it = pimpl_->infotab.find(c);
+	if (it != pimpl_->infotab.end())
+		return it->second.lyxname;
 	return "black";
 }
 
 
-size_t LColor::size() const
+bool LColor::setColor(LColor::color col, string const & x11name)
 {
-	return pimpl_->infotab.size();
-}
-
-
-void LColor::setColor(LColor::color col, string const & x11name)
-{
-	Pimpl::InfoTab::iterator iti = pimpl_->infotab.find(col);
-	if (iti != pimpl_->infotab.end()) {
-		iti->second.x11name = x11name;
-		return;
+	Pimpl::InfoTab::iterator it = pimpl_->infotab.find(col);
+	if (it == pimpl_->infotab.end()) {
+		lyxerr << "Color " << col << " not found in database."
+		       << std::endl;
+		return false;
 	}
-	lyxerr << "LyX internal error: color and such." << endl;
-	BOOST_ASSERT(false);
-}
-
-
-bool LColor::setColor(string const & lyxname, string const & x11name)
-{
-	color col = getFromLyXName(lyxname);
 
 	// "inherit" is returned for colors not in the database
 	// (and anyway should not be redefined)
-	if (col == inherit || col == ignore) {
-		lyxerr << "Color " << lyxname << " is undefined or may not be"
-			" redefined" << endl;
+	if (col == none || col == inherit || col == ignore) {
+		lyxerr << "Color " << getLyXName(col)
+		       << " may not be redefined" << endl;
 		return false;
 	}
-	setColor(col, x11name);
+	
+	it->second.x11name = x11name;
 	return true;
 }
 
 
 LColor::color LColor::getFromGUIName(string const & guiname) const
 {
-	Pimpl::InfoTab::const_iterator ici = pimpl_->infotab.begin();
+	Pimpl::InfoTab::const_iterator it = pimpl_->infotab.begin();
 	Pimpl::InfoTab::const_iterator end = pimpl_->infotab.end();
-	for (; ici != end; ++ici) {
-		if (!compare_ascii_no_case(_(ici->second.guiname), guiname))
-			return static_cast<LColor::color>(ici->first);
+	for (; it != end; ++it) {
+		if (!compare_ascii_no_case(_(it->second.guiname), guiname))
+			return static_cast<LColor::color>(it->first);
 	}
 	return LColor::inherit;
 }
 
 
+void LColor::addColor(LColor::color c, string const & lyxname) const
+{
+	ColorEntry ce = { c, "", "", "", lyxname.c_str() };
+	pimpl_->fill(ce);
+}
+
+
 LColor::color LColor::getFromLyXName(string const & lyxname) const
 {
+	if (pimpl_->transform.find(lyxname) == pimpl_->transform.end())
+		addColor(static_cast<color>(pimpl_->infotab.size()), lyxname);
+
 	return static_cast<LColor::color>(pimpl_->transform[lyxname]);
 }
 
diff --git a/src/LColor.h b/src/LColor.h
index 40c656db88..8c84cd6956 100644
--- a/src/LColor.h
+++ b/src/LColor.h
@@ -193,45 +193,30 @@ public:
 	///
 	void operator=(LColor const &);
 
-	///
-	void fill(LColor::color c,
-		  std::string const & lyxname,
-		  std::string const & x11name = std::string(),
-		  std::string const & latexname = std::string(),
-		  std::string const & guiname = std::string());
-
-	/// set the given LyX color to the color defined by the X11 name given
-	void setColor(LColor::color col, std::string const & x11name);
-	/// set the given LyX color to the color defined by the X11 name given
-	bool setColor(std::string const & lyxname, std::string const & x11name);
-
-	/// Get GUI name of color
+	/** set the given LyX color to the color defined by the X11 name given
+	 *  \returns true if successful.
+	 */
+	bool setColor(LColor::color col, std::string const & x11name);
+
+	/// Get the GUI name of \c color.
 	std::string const getGUIName(LColor::color c) const;
-	///
-	std::string const getGUIName(std::string const & s) const;
 
-	/// Get X11 name of color
+	/// Get the X11 name of \c color.
 	std::string const getX11Name(LColor::color c) const;
-	///
-	std::string const getX11Name(std::string const & s) const;
 
-	/// Get LaTeX name of color
+	/// Get the LaTeX name of \c color.
 	std::string const getLaTeXName(LColor::color c) const;
-	///
-	std::string const getLaTeXName(std::string const & s) const;
 
-	/// Get LyX name of color
+	/// Get the LyX name of \c color.
 	std::string const getLyXName(LColor::color c) const;
-	/// (string-to-string version not needed as it is identity)
-
-	///
-	size_t size() const;
 
-	/// get the color from the GUI name
+	/// \returns the LColor::color associated with the GUI name.
 	LColor::color getFromGUIName(std::string const & guiname) const;
-	/// get the color from the LyX name
+	/// \returns the LColor::color associated with the LyX name.
 	LColor::color getFromLyXName(std::string const & lyxname) const;
 private:
+	///
+	void addColor(LColor::color c, std::string const & lyxname) const;
 	///
 	struct Pimpl;
 	///
diff --git a/src/bufferparams.C b/src/bufferparams.C
index 2f3d5147cb..0f119fe573 100644
--- a/src/bufferparams.C
+++ b/src/bufferparams.C
@@ -333,20 +333,22 @@ string const BufferParams::readToken(LyXLex & lex, string const & token)
 			string const tok = lex.getString();
 			if (tok == "\\end_branch")
 				break;
+			Branch * branch_ptr = branchlist().find(branch);
 			if (tok == "\\selected") {
 				lex.nextToken();
-				branchlist().setSelected(branch, lex.getInteger());
+				if (branch_ptr)
+					branch_ptr->setSelected(lex.getInteger());
 			}
 			// not yet operational
 			if (tok == "\\color") {
 				lex.nextToken();
 				string color = lex.getString();
-				branchlist().setColor(branch, color);
+				if (branch_ptr)
+					branch_ptr->setColor(color);
 				// Update also the LColor table:
 				if (color == "none")
 					color = lcolor.getX11Name(LColor::background);
-				lcolor.fill(static_cast<LColor::color>(lcolor.size()),
-						branch, color);
+				lcolor.setColor(lcolor.getFromLyXName(branch), color);
 			}
 		}
 	} else if (token == "\\author") {
diff --git a/src/factory.C b/src/factory.C
index 387a3a5f02..e6f75634a4 100644
--- a/src/factory.C
+++ b/src/factory.C
@@ -111,7 +111,7 @@ InsetOld * createInset(FuncRequest const & cmd)
 		string arg = cmd.getArg(0);
 		if (arg.empty())
 			arg = "none";
-		return new InsetBranch(params, arg);
+		return new InsetBranch(params, InsetBranchParams(arg));
 	}
 
 	case LFUN_INSET_ERT:
@@ -419,7 +419,8 @@ InsetOld * readInset(LyXLex & lex, Buffer const & buf)
 			CharStyles::iterator found_cs = tclass.charstyle(s);
 			inset.reset(new InsetCharStyle(buf.params(), found_cs));
 		} else if (tmptok == "Branch") {
-			inset.reset(new InsetBranch(buf.params(), string()));
+			inset.reset(new InsetBranch(buf.params(),
+						    InsetBranchParams()));
 		} else if (tmptok == "Include") {
 			InsetCommandParams p("Include");
 			inset.reset(new InsetInclude(p));
diff --git a/src/frontends/controllers/ChangeLog b/src/frontends/controllers/ChangeLog
index 8bbbec0bb8..460630a994 100644
--- a/src/frontends/controllers/ChangeLog
+++ b/src/frontends/controllers/ChangeLog
@@ -1,3 +1,10 @@
+2003-12-14  Angus Leeming  <leeming@lyx.org>
+
+	* ControlBranch.[Ch] (branchlist): new member function.
+
+	* ControlDocument.C (setBranchColor): removed.
+	(apply): update LColor to accommodate the new BranchList colors.
+
 2003-12-14  Jürgen Spitzmüller  <j.spitzmueller@gmx.de>
 
 	* ControlMinipage.[Ch]: remove.
@@ -8,13 +15,6 @@
 	* ControlMath.C (dispatchSubscript, dispatchSuperscript): enable
 	the math panel sub/superscript buttons once again.
 
-2003-12-12  Angus Leeming  <leeming@lyx.org>
-
-	* ControlBranch.[Ch] (branchlist): new member function.
-
-	* ControlDocument.C (setBranchColor): remove redundant call to
-	LColor::setColor.
-
 2003-12-11  Angus Leeming  <leeming@lyx.org>
 
 	* ControlCommand.C (initialiseParams): changes due to the changed
diff --git a/src/frontends/controllers/ControlBranch.C b/src/frontends/controllers/ControlBranch.C
index 10f9811a36..4185fd4de6 100644
--- a/src/frontends/controllers/ControlBranch.C
+++ b/src/frontends/controllers/ControlBranch.C
@@ -12,11 +12,16 @@
 #include <config.h>
 
 #include "ControlBranch.h"
+
+#include "buffer.h"
+#include "bufferparams.h"
+#include "BranchList.h"
 #include "funcrequest.h"
 #include "insets/insetbranch.h"
 
 
 using std::string;
+using std::vector;
 
 
 ControlBranch::ControlBranch(Dialog & parent)
@@ -44,3 +49,9 @@ void ControlBranch::dispatchParams()
 	string const lfun = InsetBranchMailer::params2string(params());
 	kernel().dispatch(FuncRequest(LFUN_INSET_APPLY, lfun));
 }
+
+
+BranchList const & ControlBranch::branchlist() const
+{
+	return kernel().buffer().params().branchlist();
+}
diff --git a/src/frontends/controllers/ControlBranch.h b/src/frontends/controllers/ControlBranch.h
index b364ef5f67..ef364107ab 100644
--- a/src/frontends/controllers/ControlBranch.h
+++ b/src/frontends/controllers/ControlBranch.h
@@ -16,8 +16,10 @@
 
 #include "Dialog.h"
 
+class BranchList;
 class InsetBranchParams;
 
+
 class ControlBranch : public Dialog::Controller {
 public:
 	///
@@ -32,9 +34,10 @@ public:
 	virtual bool isBufferDependent() const { return true; }
 	///
 	InsetBranchParams & params() { return *params_.get(); }
-	///
 	InsetBranchParams const & params() const { return *params_.get(); }
 	///
+	BranchList const & branchlist() const;
+
 private:
 	///
 	boost::scoped_ptr<InsetBranchParams> params_;
diff --git a/src/frontends/controllers/ControlDocument.C b/src/frontends/controllers/ControlDocument.C
index 302d2d84cd..cbab160ad2 100644
--- a/src/frontends/controllers/ControlDocument.C
+++ b/src/frontends/controllers/ControlDocument.C
@@ -13,6 +13,7 @@
 #include "ControlDocument.h"
 #include "ViewBase.h"
 
+#include "BranchList.h"
 #include "buffer.h"
 #include "buffer_funcs.h"
 #include "bufferparams.h"
@@ -22,11 +23,11 @@
 #include "gettext.h"
 #include "iterators.h"
 #include "language.h"
+#include "LColor.h"
 #include "lyxtextclasslist.h"
 #include "paragraph.h"
 #include "funcrequest.h"
 #include "lfuns.h"
-#include "LColor.h"
 
 #include "frontends/Alert.h"
 #include "frontends/LyXView.h"
@@ -85,6 +86,22 @@ void ControlDocument::apply()
 
 	lv_.message(_("Document settings applied"));
 
+	// branches
+	BranchList & branchlist = params().branchlist();
+	BranchList::const_iterator it = branchlist.begin();
+	BranchList::const_iterator const end = branchlist.end();
+	for (; it != end; ++it) {
+		string const & current_branch = it->getBranch();
+		Branch const * branch = branchlist.find(current_branch);
+		string x11hexname = branch->getColor();
+		// check that we have a valid color!
+		if (x11hexname.empty() || x11hexname[0] != '#')
+			x11hexname = lcolor.getX11Name(LColor::background);
+		// display the new color
+		string const str = current_branch  + ' ' + x11hexname;
+		lv_.dispatch(FuncRequest(LFUN_SET_COLOR, str));
+	}
+
 	// Open insets of selected branches, close deselected ones
 	// Currently only top-level insets in buffer handled (bug).
 	ParIterator pit = buffer()->par_iterator_begin();
@@ -121,14 +138,6 @@ void ControlDocument::setLanguage()
 }
 
 
-void ControlDocument::setBranchColor(string const & branch, string const & hex)
-{
-	lcolor.setColor(branch, hex);
-	string const s = branch + ' ' + hex;
-	lv_.dispatch(FuncRequest(LFUN_SET_COLOR, s));
-}
-
-
 void ControlDocument::classApply()
 {
 	BufferParams & params = buffer()->params();
diff --git a/src/frontends/controllers/ControlDocument.h b/src/frontends/controllers/ControlDocument.h
index dcd47bfe60..3661f93294 100644
--- a/src/frontends/controllers/ControlDocument.h
+++ b/src/frontends/controllers/ControlDocument.h
@@ -34,8 +34,6 @@ public:
 	///
 	void setLanguage();
 	///
-	void setBranchColor(std::string const & branch, std::string const & hex);
-	///
 	LyXTextClass textClass();
 	///
 	BufferParams & params();
diff --git a/src/frontends/qt2/ChangeLog b/src/frontends/qt2/ChangeLog
index cb636b376c..0e86eb4e32 100644
--- a/src/frontends/qt2/ChangeLog
+++ b/src/frontends/qt2/ChangeLog
@@ -1,3 +1,10 @@
+2003-12-14  Angus Leeming  <leeming@lyx.org>
+
+	* QBranch.C: use the List interface to BranchList, simplifying the code.
+
+	* QDocument.C:
+	* QDocumentDialog.C: overhaul the branchlist code.
+
 2003-12-14  Jürgen Spitzmüller  <j.spitzmueller@gmx.de>
 
 	* QMinipage.[Ch]:
diff --git a/src/frontends/qt2/QBranch.C b/src/frontends/qt2/QBranch.C
index f08532a0d5..6a5557514c 100644
--- a/src/frontends/qt2/QBranch.C
+++ b/src/frontends/qt2/QBranch.C
@@ -11,22 +11,21 @@
 #include <config.h>
 
 #include "debug.h"
-#include "qt_helpers.h"
-#include "ControlBranch.h"
-#include "insets/insetbranch.h"
-#include "support/lstrings.h"
+#include "BranchList.h"
 
-#include <qcombobox.h>
-#include <qpushbutton.h>
+#include "controllers/ControlBranch.h"
+
+#include "insets/insetbranch.h"
 
-#include "QBranchDialog.h"
 #include "QBranch.h"
+#include "QBranchDialog.h"
 #include "Qt2BC.h"
+#include "qt_helpers.h"
 
-using lyx::support::getVectorFromString;
+#include <qcombobox.h>
+#include <qpushbutton.h>
 
 using std::string;
-using std::vector;
 
 
 typedef QController<ControlBranch, QView<QBranchDialog> > base_class;
@@ -39,16 +38,8 @@ QBranch::QBranch(Dialog & parent)
 
 void QBranch::build_dialog()
 {
-	string all_branches(controller().params().branchlist.allBranches());
-
 	dialog_.reset(new QBranchDialog(this));
 
-	std::vector<string> all = getVectorFromString(all_branches, "|");
-	for (unsigned i = 0; i < all.size(); ++i) {
-		QString const bname = toqstr(all[i].c_str());
-		dialog_->branchCO->insertItem(bname);
-		}
-
 	bcview().setOK(dialog_->okPB);
 	bcview().setCancel(dialog_->closePB);
 }
@@ -56,21 +47,25 @@ void QBranch::build_dialog()
 
 void QBranch::update_contents()
 {
-	// re-read branch list
+	typedef BranchList::const_iterator const_iterator;
+
+	BranchList const & branchlist = controller().branchlist();
+	string const cur_branch = controller().params().branch;
+
 	dialog_->branchCO->clear();
-	string all_branches(controller().params().branchlist.allBranches());
-	string cur_branch(controller().params().branch);
-	unsigned int cur_item = 0;
-	std::vector<string> all = getVectorFromString(all_branches, "|");
-	for (unsigned i = 0; i < all.size(); ++i) {
-		QString const bname = toqstr(all[i].c_str());
-		dialog_->branchCO->insertItem(bname);
-		if (bname == toqstr(cur_branch))
-			cur_item = i;
+
+	const_iterator const begin = branchlist.begin();
+	const_iterator const end = branchlist.end();
+	int id = 1;
+	int count = 1;
+	for (const_iterator it = begin; it != end; ++it, ++count) {
+		string const & branch = it->getBranch();
+		dialog_->branchCO->insertItem(toqstr(branch));
+
+		if (cur_branch == branch)
+			id = count;
 	}
-	// set to current item. A better idea anyone?
-	if (all_branches.find(cur_branch) != string::npos && cur_branch != "none")
-		dialog_->branchCO->setCurrentItem(cur_item);
+	dialog_->branchCO->setCurrentItem(id);
 }
 
 
diff --git a/src/frontends/qt2/QDocument.C b/src/frontends/qt2/QDocument.C
index af27a2c706..89c2de1fd4 100644
--- a/src/frontends/qt2/QDocument.C
+++ b/src/frontends/qt2/QDocument.C
@@ -27,7 +27,6 @@
 #include "support/lstrings.h"
 #include "lyxtextclasslist.h"
 #include "floatplacement.h"
-#include "LColor.h"
 
 #include <qpushbutton.h>
 #include <qmultilineedit.h>
@@ -388,24 +387,7 @@ void QDocument::apply()
 
 	params.footskip = widgetsToLength(m->footskipLE, m->footskipUnit);
 
-	// branches
-	string const all_branches = params.branchlist().allBranches();
-	if (!all_branches.empty()) {
-		std::vector<string> all = getVectorFromString(all_branches, "|");
-		for (unsigned i = 0; i < all.size(); ++i) {
-			string const current_branch = all[i].c_str();
-			string x11hexname = params.branchlist().getColor(current_branch);
-			// check that we have a valid color!
-			if (x11hexname.empty() || x11hexname[0] != '#')
-				x11hexname = lcolor.getX11Name(LColor::background);
-			// display the new color
-			controller().setBranchColor(current_branch, x11hexname);
-		}
-	}
-	if (branchlist_.empty())
-		branchlist_ = params.branchlist();
 	params.branchlist() = branchlist_;
-	branchlist_.clear();
 }
 
 
@@ -654,6 +636,7 @@ void QDocument::update_contents()
 		params.footskip, defaultUnit);
 
 	// branches
+	branchlist_ = params.branchlist();
 	dialog_->updateBranchView();
 }
 
diff --git a/src/frontends/qt2/QDocumentDialog.C b/src/frontends/qt2/QDocumentDialog.C
index 330d2577ca..938ccaa62d 100644
--- a/src/frontends/qt2/QDocumentDialog.C
+++ b/src/frontends/qt2/QDocumentDialog.C
@@ -10,20 +10,21 @@
 
 #include <config.h>
 
+#include "bufferparams.h"
 #include "debug.h"
-#include "qt_helpers.h"
+#include "lyxrc.h"
+
+#include "controllers/ControlDocument.h"
+
+#include "support/lstrings.h"
 
-#include "ControlDocument.h"
 #include "QDocument.h"
 #include "QDocumentDialog.h"
 
-#include "panelstack.h"
 #include "floatplacement.h"
-#include "LColor.h"
-
-#include "support/lstrings.h"
-#include "bufferparams.h"
-#include "lyxrc.h"
+#include "lengthcombo.h"
+#include "panelstack.h"
+#include "qt_helpers.h"
 
 #include <qlabel.h>
 #include <qmultilineedit.h>
@@ -36,12 +37,9 @@
 #include <qpixmap.h>
 #include <qcolor.h>
 #include <qcolordialog.h>
-#include "lengthcombo.h"
 
 using lyx::support::token;
-using lyx::support::getVectorFromString;
 
-using std::vector;
 using std::string;
 
 
@@ -424,43 +422,34 @@ void QDocumentDialog::updateNumbering()
 
 void QDocumentDialog::updateBranchView()
 {
-	ControlDocument & cntrl = form_->controller();
-	BufferParams & params = cntrl.params();
-
-	string const all_branches = params.branchlist().allBranches();
 	branchesModule->branchesLV->clear();
-	if (!all_branches.empty()) {
-		std::vector<string> all = getVectorFromString(all_branches, "|");
-		for (unsigned i = 0; i < all.size(); ++i) {
-			QString const bname = toqstr(all[i].c_str());
-			QString const sel =
-				(params.branchlist().selected(fromqstr(bname))) ? qt_("Yes") : qt_("No");
-			QListViewItem * newItem =
-				new QListViewItem(branchesModule->branchesLV, bname, sel);
-			QColor itemcolor;
-			string x11hexname = params.branchlist().getColor(fromqstr(bname));
-			if (x11hexname[0] == '#')
-				itemcolor.setNamedColor(toqstr(x11hexname));
-			if (itemcolor.isValid()) {
-				QPixmap coloritem(30, 10);
-				coloritem.fill(itemcolor);
-				newItem->setPixmap(2, coloritem);
-			}
+
+	BranchList::const_iterator it = form_->branchlist_.begin();
+	BranchList::const_iterator const end = form_->branchlist_.end();
+	for (; it != end; ++it) {
+		QString const bname = toqstr(it->getBranch());
+		QString const sel = it->getSelected() ? qt_("Yes") : qt_("No");
+		QListViewItem * newItem =
+			new QListViewItem(branchesModule->branchesLV, bname, sel);
+		string const x11hexname = it->getColor();
+		QColor itemcolor;
+		if (x11hexname[0] == '#')
+			itemcolor.setNamedColor(toqstr(x11hexname));
+		if (itemcolor.isValid()) {
+			QPixmap coloritem(30, 10);
+			coloritem.fill(itemcolor);
+			newItem->setPixmap(2, coloritem);
 		}
 	}
-	form_->branchlist_ = params.branchlist();
 	form_->changed();
 }
 
 
 void QDocumentDialog::addBranchPressed()
 {
-	ControlDocument & cntrl = form_->controller();
-	BufferParams & params = cntrl.params();
-
 	QString const new_branch = branchesModule->newBranchLE->text();
 	if (!new_branch.isEmpty()) {
-		params.branchlist().add(fromqstr(new_branch));
+		form_->branchlist_.add(fromqstr(new_branch));
 		branchesModule->newBranchLE->clear();
 		updateBranchView();
 	}
@@ -469,16 +458,13 @@ void QDocumentDialog::addBranchPressed()
 
 void QDocumentDialog::deleteBranchPressed()
 {
-	ControlDocument & cntrl = form_->controller();
-	BufferParams & params = cntrl.params();
-
 	QListViewItem * selItem =
 		branchesModule->branchesLV->selectedItem();
 	QString sel_branch;
 	if (selItem != 0)
 		sel_branch = selItem->text(0);
 	if (sel_branch) {
-		params.branchlist().remove(fromqstr(sel_branch));
+		form_->branchlist_.remove(fromqstr(sel_branch));
 		branchesModule->newBranchLE->clear();
 		updateBranchView();
 	}
@@ -501,28 +487,23 @@ void QDocumentDialog::branchDoubleClicked(QListViewItem * selItem)
 
 void QDocumentDialog::toggleBranch(QListViewItem * selItem)
 {
-	ControlDocument & cntrl = form_->controller();
-	BufferParams & params = cntrl.params();
+	if (selItem == 0)
+		return;
 
-	QString sel_branch;
-	if (selItem != 0)
-		sel_branch = selItem->text(0);
+	QString sel_branch = selItem->text(0);
 	if (sel_branch) {
-		bool selected = false;
-		if (selItem->text(1) == qt_("Yes"))
-			selected = true;
-		params.branchlist().setSelected(fromqstr(sel_branch), !selected);
-		branchesModule->newBranchLE->clear();
-		updateBranchView();
+		bool const selected = selItem->text(1) == qt_("Yes");
+		Branch * branch = form_->branchlist_.find(fromqstr(sel_branch));
+		if (branch && branch->setSelected(!selected)) {
+			branchesModule->newBranchLE->clear();
+			updateBranchView();
+		}
 	}
 }
 
 
 void QDocumentDialog::toggleBranchColor()
 {
-	ControlDocument & cntrl = form_->controller();
-	BufferParams & params = cntrl.params();
-
 	QListViewItem * selItem =
 		branchesModule->branchesLV->selectedItem();
 	QString sel_branch;
@@ -531,13 +512,18 @@ void QDocumentDialog::toggleBranchColor()
 	if (sel_branch) {
 		QColor initial;
 		string current_branch = fromqstr(sel_branch);
-		string x11hexname = params.branchlist().getColor(current_branch);
+		Branch * branch =
+			form_->branchlist_.find(current_branch);
+		if (!branch)
+			return;
+
+		string x11hexname = branch->getColor();
 		if (x11hexname[0] == '#')
 			initial.setNamedColor(toqstr(x11hexname));
 		QColor ncol(QColorDialog::getColor(initial));
 		if (ncol.isValid()){
 			// add the color to the branchlist
-			params.branchlist().setColor(current_branch, fromqstr(ncol.name()));
+			branch->setColor(fromqstr(ncol.name()));
 			branchesModule->newBranchLE->clear();
 			updateBranchView();
 		}
diff --git a/src/frontends/xforms/ChangeLog b/src/frontends/xforms/ChangeLog
index 78db2944ac..919ebf601e 100644
--- a/src/frontends/xforms/ChangeLog
+++ b/src/frontends/xforms/ChangeLog
@@ -1,3 +1,14 @@
+2003-12-14  Angus Leeming  <leeming@lyx.org>
+
+	* FormBranch.C: use the List interface to BranchList, simplifying the
+	code.
+
+	* FormDocument.C: overhaul the branch code, using the List
+	interface to BranchList and factoring repeated blocks into two
+	new helper functions.
+
+	* lyx_gui.C (update_color): ensure the GC is up to date.
+
 2003-12-14  Jürgen Spitzmüller  <j.spitzmueller@gmx.de>
 
 	* FormMinipage.[Ch]:
diff --git a/src/frontends/xforms/FormBranch.C b/src/frontends/xforms/FormBranch.C
index face8b5643..c2badd04ea 100644
--- a/src/frontends/xforms/FormBranch.C
+++ b/src/frontends/xforms/FormBranch.C
@@ -12,16 +12,17 @@
 #include <config.h>
 
 #include "FormBranch.h"
-#include "ControlBranch.h"
 #include "forms/form_branch.h"
-
 #include "xformsBC.h"
 
+#include "BranchList.h"
+
+#include "controllers/ControlBranch.h"
+
 #include "insets/insetbranch.h"
 
 #include "lyx_forms.h"
 
-
 using std::string;
 
 
@@ -34,12 +35,8 @@ FormBranch::FormBranch(Dialog & parent)
 
 void FormBranch::build()
 {
-	string all_branches(controller().params().branchlist.allBranches());
-
 	dialog_.reset(build_branch(this));
 
-	fl_addto_choice(dialog_->choice_branch, all_branches.c_str());
-
 	bcview().setOK(dialog_->button_ok);
 	bcview().setApply(dialog_->button_apply);
 	bcview().setCancel(dialog_->button_cancel);
@@ -48,20 +45,30 @@ void FormBranch::build()
 
 void FormBranch::update()
 {
-	// Make changes in all_branches propagate within session:
-	string all_branches(controller().params().branchlist.allBranches());
+	typedef BranchList::const_iterator const_iterator;
+
+	BranchList const & branchlist = controller().branchlist();
+	string const cur_branch = controller().params().branch;
+
 	fl_clear_choice(dialog_->choice_branch);
-	fl_addto_choice(dialog_->choice_branch, all_branches.c_str());
 
-	string branch(controller().params().branch);
-	if (all_branches.find(branch) != string::npos && branch != "none")
-		fl_set_choice_text(dialog_->choice_branch, branch.c_str());
+	const_iterator const begin = branchlist.begin();
+	const_iterator const end = branchlist.end();
+	int id = 1;
+	int count = 1;
+	for (const_iterator it = begin; it != end; ++it, ++count) {
+		string const & branch = it->getBranch();
+		fl_addto_choice(dialog_->choice_branch, branch.c_str());
+
+		if (cur_branch == branch)
+			id = count;
+	}
+	fl_set_choice(dialog_->choice_branch, id);
 }
 
 
 void FormBranch::apply()
 {
 	string const type = fl_get_choice_text(dialog_->choice_branch);
-
 	controller().params().branch = type;
 }
diff --git a/src/frontends/xforms/FormDocument.C b/src/frontends/xforms/FormDocument.C
index a1c14a7742..54d6411505 100644
--- a/src/frontends/xforms/FormDocument.C
+++ b/src/frontends/xforms/FormDocument.C
@@ -34,7 +34,6 @@
 #include "Bullet.h"
 #include "bufferparams.h"
 #include "language.h"
-#include "LColor.h"
 #include "lyxrc.h"
 #include "lyxtextclasslist.h"
 #include "tex-strings.h"
@@ -670,123 +669,122 @@ ButtonPolicy::SMInput FormDocument::input(FL_OBJECT * ob, long)
 }
 
 
-void FormDocument::branch_input(FL_OBJECT * ob)
+void FormDocument::rebuild_all_branches_browser()
 {
-	BufferParams & params = controller().params();
-	std::vector<string> vec;
+	typedef BranchList::const_iterator const_iterator;
 
+	fl_clear_browser(branch_->browser_all_branches);
+
+	const_iterator const begin = branchlist_.begin();
+	const_iterator const end = branchlist_.end();
+	for (const_iterator it = begin; it != end; ++it) {
+		fl_addto_browser(branch_->browser_all_branches,
+				 it->getBranch().c_str());
+	}
+}
+
+
+void FormDocument::rebuild_selected_branches_browser()
+{
+	typedef BranchList::const_iterator const_iterator;
+
+	fl_clear_browser(branch_->browser_selection);
+
+	const_iterator const begin = branchlist_.begin();
+	const_iterator const end = branchlist_.end();
+	for (const_iterator it = begin; it != end; ++it) {
+		if (it->getSelected())
+			fl_addto_browser(branch_->browser_selection,
+					 it->getBranch().c_str());
+	}
+}
+
+
+namespace {
+
+RGBColor get_current_color(FL_OBJECT * browser, BranchList const & branchlist)
+{
+	BOOST_ASSERT(browser && browser->objclass == FL_BROWSER);
+
+	RGBColor color;
+
+	int const i = fl_get_browser(browser);
+	string const branch_name = fl_get_browser_line(browser, i);
+	Branch const * branch = branchlist.find(branch_name);
+	if (!branch)
+		return color;
+
+	string const x11hexname = branch->getColor();
+	if (x11hexname[0] == '#') {
+		color = RGBColor(x11hexname);
+	} else{
+		fl_getmcolor(FL_COL1, &color.r, &color.g, &color.b);
+	}
+	return color;
+}
+
+} // namespace anon
+
+
+void FormDocument::branch_input(FL_OBJECT * ob)
+{
 	if (ob == branch_->button_add_branch) {
-		string new_branch = fl_get_input(branch_->input_all_branches);
-		if (!new_branch.empty()) {
-			params.branchlist().add(new_branch);
+		string const new_branch =
+			getString(branch_->input_all_branches);
+
+		if (!new_branch.empty() && branchlist_.add(new_branch)) {
+
 			fl_set_input(branch_->input_all_branches, "");
-			// Update branch list
-			string const all_branches = params.branchlist().allBranches();
-			fl_clear_browser(branch_->browser_all_branches);
-			vec = getVectorFromString(all_branches, "|");
-			for (unsigned i = 0; i < vec.size(); ++i) {
-				fl_addto_browser(branch_->browser_all_branches,
-								vec[i].c_str());
-			}
-			LColor::color c = static_cast<LColor::color>(lcolor.size());
-			lcolor.fill(c, new_branch, lcolor.getX11Name(LColor::background));
+			rebuild_all_branches_browser();
 		}
 
 	} else if (ob == branch_->button_remove_branch) {
 		unsigned i = fl_get_browser(branch_->browser_all_branches);
 		string const current_branch =
 			fl_get_browser_line(branch_->browser_all_branches, i);
-		if (!current_branch.empty()) {
-			params.branchlist().remove(current_branch);
-			// Update branch list
-			string const all_branches = params.branchlist().allBranches();
-			fl_clear_browser(branch_->browser_all_branches);
-			vec = getVectorFromString(all_branches, "|");
-			for (unsigned i = 0; i < vec.size(); ++i) {
-				fl_addto_browser(branch_->browser_all_branches,
-								vec[i].c_str());
-			}
-			// Update selected-list...
-			string const all_selected = params.branchlist().allSelected();
-			fl_clear_browser(branch_->browser_selection);
-			vec = getVectorFromString(all_selected, "|");
-			for (unsigned i = 0; i < vec.size(); ++i) {
-				fl_addto_browser(branch_->browser_selection, vec[i].c_str());
-			}
-		}
-	} else if (ob == branch_->button_select) {
-		unsigned i = fl_get_browser(branch_->browser_all_branches);
-		string const current_branch =
-			fl_get_browser_line(branch_->browser_all_branches, i);
-		if (!current_branch.empty()) {
-			fl_clear_browser(branch_->browser_selection);
-			params.branchlist().setSelected(current_branch, true);
-			string const all_selected = params.branchlist().allSelected();
-			vec = getVectorFromString(all_selected, "|");
-			for (unsigned i = 0; i < vec.size(); ++i) {
-				fl_addto_browser(branch_->browser_selection,
-							vec[i].c_str());
-			}
-		}
-	} else if (ob == branch_->button_deselect) {
-		unsigned i = fl_get_browser(branch_->browser_selection);
-		string const current_sel =
-			fl_get_browser_line(branch_->browser_selection, i);
-		if (!current_sel.empty()) {
-			fl_clear_browser(branch_->browser_selection);
-			params.branchlist().setSelected(current_sel, false);
-			string const all_selected = params.branchlist().allSelected();
-			vec = getVectorFromString(all_selected, "|");
-			for (unsigned i = 0; i < vec.size(); ++i) {
-				fl_addto_browser(branch_->browser_selection,
-							vec[i].c_str());
-			}
+		if (!current_branch.empty() &&
+		    branchlist_.remove(current_branch)) {
+
+			rebuild_all_branches_browser();
+			rebuild_selected_branches_browser();
 		}
-	} else if (ob == branch_->button_modify) {
-		unsigned i = fl_get_browser(branch_->browser_all_branches);
+
+	} else if (ob == branch_->button_select ||
+		   ob == branch_->button_deselect) {
+
+		bool const selected = ob == branch_->button_select;
+
+		int const i = fl_get_browser(branch_->browser_all_branches);
 		string const current_branch =
 			fl_get_browser_line(branch_->browser_all_branches, i);
 
-		RGBColor before;
-		string x11hexname = params.branchlist().getColor(current_branch);
-		if (x11hexname[0] == '#') {
-			before = RGBColor(x11hexname);
-		} else{
-			fl_getmcolor(FL_COL1, &before.r, &before.g, &before.b);
-		}
+		Branch * branch = branchlist_.find(current_branch);
+		
+		if (branch && branch->setSelected(selected))
+			rebuild_selected_branches_browser();
 
-		RGBColor col = picker_->requestColor(before);
-		if (before != col) {
-			fl_mapcolor(GUI_COLOR_CHOICE, col.r, col.g, col.b);
+	} else if (ob == branch_->button_modify) {
+		RGBColor const before =
+			get_current_color(branch_->browser_all_branches,
+					  branchlist_);
+		RGBColor const after = picker_->requestColor(before);
+		if (before != after) {
+			fl_mapcolor(GUI_COLOR_CHOICE,
+				    after.r, after.g, after.b);
 			fl_redraw_object(branch_->button_color);
-			// Figure out here how to stash the new colour into the
-			// LyX colour database.
-
-			x11hexname = X11hexname(col);
-
-			// current_branch already in database
-			LColor::color c = lcolor.getFromLyXName(current_branch);
-			lcolor.setColor(current_branch, x11hexname);
-			// Make sure that new colour is also displayed ;-)
-			lyxColorHandler->getGCForeground(c);
-			lyxColorHandler->updateColor(c);
-			// what about system_lcolor?
-			// Here set colour in BranchList:
-			params.branchlist().setColor(current_branch, x11hexname);
+
+			string const branch_name =
+				getString(branch_->browser_all_branches);
+			Branch * branch = branchlist_.find(branch_name);
+			if (branch)
+				branch->setColor(X11hexname(after));
 		}
+
 	} else if (ob == branch_->browser_all_branches) {
-		unsigned i = fl_get_browser(branch_->browser_all_branches);
-		string const current_branch =
-			fl_get_browser_line(branch_->browser_all_branches, i);
-		// make button_color track selected branch:
+		RGBColor rgb =
+			get_current_color(branch_->browser_all_branches,
+					  branchlist_);
 
-		RGBColor rgb;
-		string x11hexname = params.branchlist().getColor(current_branch);
-		if (x11hexname[0] == '#') {
-			rgb = RGBColor(x11hexname);
-		} else {
-			fl_getmcolor(FL_COL1, &rgb.r, &rgb.g, &rgb.b);
-		}
 		fl_mapcolor(GUI_COLOR_CHOICE, rgb.r, rgb.g, rgb.b);
 		fl_redraw_object(branch_->button_color);
 	}
@@ -798,8 +796,6 @@ void FormDocument::branch_input(FL_OBJECT * ob)
 		(fl_get_browser(branch_->browser_all_branches) > 0));
 	setEnabled(branch_->button_modify,
 		(fl_get_browser(branch_->browser_all_branches) > 0));
-
-	branchlist_ = params.branchlist();
 }
 
 
@@ -1029,11 +1025,7 @@ void FormDocument::bullets_apply(BufferParams & params)
 
 void FormDocument::branch_apply(BufferParams & params)
 {
-	BufferParams & prms = controller().params();
-	if (branchlist_.empty())
-		branchlist_ = prms.branchlist();
 	params.branchlist() = branchlist_;
-	branchlist_.clear();
 }
 
 
@@ -1307,39 +1299,28 @@ void FormDocument::branch_update(BufferParams const & params)
 	if (!branch_.get())
 		return;
 
-	string const all_branches = params.branchlist().allBranches();
-	fl_clear_browser(branch_->browser_all_branches);
-	string current_branch("none");
+	branchlist_ = params.branchlist();
 
-	if (!all_branches.empty()) {
-		std::vector<string> vec = getVectorFromString(all_branches, "|");
-		for (unsigned i = 0; i < vec.size(); ++i) {
-			fl_addto_browser(branch_->browser_all_branches, vec[i].c_str());
-		}
-		fl_select_browser_line(branch_->browser_all_branches, 1);
-		if (!vec.empty())
-			current_branch =
-				fl_get_browser_line(branch_->browser_all_branches, 1);
-		else
-			current_branch = "none";
-	}
+	rebuild_all_branches_browser();
+	string const current_branch =
+		fl_get_browser_maxline(branch_->browser_all_branches) == 0 ?
+		"none" :
+		fl_get_browser_line(branch_->browser_all_branches, 1);
 
 	// display proper selection...
-	string const all_selected = params.branchlist().allSelected();
-	fl_clear_browser(branch_->browser_selection);
-	if (!all_selected.empty()) {
-		std::vector<string> vec = getVectorFromString(all_selected, "|");
-		for (unsigned i = 0; i < vec.size(); ++i) {
-			fl_addto_browser(branch_->browser_selection, vec[i].c_str());
-			}
-	}
+	rebuild_selected_branches_browser();
+
 	// display proper colour...
 	RGBColor rgb;
 	string x11hexname;
 	if (current_branch == "none")
 		x11hexname = "none";
-	else
-		x11hexname = params.branchlist().getColor(current_branch);
+	else {
+		Branch * branch = branchlist_.find(current_branch);
+		if (branch)
+			x11hexname = branch->getColor();
+	}
+
 	if (x11hexname[0] == '#') {
 		rgb = RGBColor(x11hexname);
 	} else {
diff --git a/src/frontends/xforms/FormDocument.h b/src/frontends/xforms/FormDocument.h
index 01fef77761..3cd5084a78 100644
--- a/src/frontends/xforms/FormDocument.h
+++ b/src/frontends/xforms/FormDocument.h
@@ -110,6 +110,9 @@ private:
 	///
 	void branch_apply(BufferParams &);
 
+	void rebuild_all_branches_browser();
+	void rebuild_selected_branches_browser();
+
 	/// Real GUI implementation.
 	boost::scoped_ptr<FD_document_paper>    paper_;
 	///
diff --git a/src/frontends/xforms/lyx_gui.C b/src/frontends/xforms/lyx_gui.C
index aff7700353..a2e8ecba19 100644
--- a/src/frontends/xforms/lyx_gui.C
+++ b/src/frontends/xforms/lyx_gui.C
@@ -366,6 +366,7 @@ string const hexname(LColor_color col)
 
 void update_color(LColor_color col)
 {
+	lyxColorHandler->getGCForeground(col);
 	lyxColorHandler->updateColor(col);
 }
 
diff --git a/src/insets/ChangeLog b/src/insets/ChangeLog
index c5f2d133c1..51311d49e9 100644
--- a/src/insets/ChangeLog
+++ b/src/insets/ChangeLog
@@ -1,3 +1,12 @@
+2003-12-12  Angus Leeming  <leeming@lyx.org>
+
+	* insetbranch.C (c-tor): takes an InsetBranchParams arg rather than
+	a string.
+	(InsetBranchParams): remove the branchlist hack.
+	(isBranchSelected): new function, used to decide whether to
+	print each output format. Also used by InsetList.
+	(read, string2params): repair my own breakage ;-)
+	
 2003-12-14  Jürgen Spitzmüller  <j.spitzmueller@gmx.de>
 
 	* insetminipage.[Ch]: remove from repository.
diff --git a/src/insets/insetbranch.C b/src/insets/insetbranch.C
index 9cc918f402..a2e67e0e3c 100644
--- a/src/insets/insetbranch.C
+++ b/src/insets/insetbranch.C
@@ -14,6 +14,7 @@
 
 #include "buffer.h"
 #include "bufferparams.h"
+#include "BranchList.h"
 #include "BufferView.h"
 #include "dispatchresult.h"
 #include "funcrequest.h"
@@ -39,13 +40,10 @@ void InsetBranch::init()
 }
 
 
-InsetBranch::InsetBranch(BufferParams const & bp, string const & label)
-	: InsetCollapsable(bp)
+InsetBranch::InsetBranch(BufferParams const & bp,
+			 InsetBranchParams const & params)
+	: InsetCollapsable(bp), params_(params)
 {
-	params_.branch = label;
-	// Hack: stash the list of all allowable branch labels from this
-	// buffer into inset's parm list as a "stowaway":
-	params_.branchlist = bp.branchlist();
 	init();
 }
 
@@ -84,10 +82,7 @@ void InsetBranch::write(Buffer const & buf, ostream & os) const
 
 void InsetBranch::read(Buffer const & buf, LyXLex & lex)
 {
-	if (lex.isOK()) {
-		lex.next();
-		params_.branch = lex.getString();
-	}
+	params_.read(lex);
 	InsetCollapsable::read(buf, lex);
 	setButtonLabel();
 }
@@ -152,44 +147,59 @@ InsetBranch::priv_dispatch(FuncRequest const & cmd,
 }
 
 
+namespace {
+
+struct SameBranch {
+	SameBranch(string const & branch_name) : bn(branch_name) {}
+	bool operator()(Branch const & branch) const
+		{ return bn == branch.getBranch(); }
+private:
+	string bn;
+};
+
+} // namespace anon
+
+
+bool InsetBranch::isBranchSelected(BranchList const & branchlist) const
+{
+	BranchList::const_iterator it = branchlist.begin();
+	BranchList::const_iterator const end = branchlist.end();
+	it = std::find_if(it, end, SameBranch(params_.branch));
+	if (it == end)
+		return false;
+	return it->getSelected();
+}
+
+
 int InsetBranch::latex(Buffer const & buf, ostream & os,
-	OutputParams const & runparams) const
+		       OutputParams const & runparams) const
 {
-	string const branch_sel = buf.params().branchlist().allSelected();
-	if (branch_sel.find(params_.branch, 0) != string::npos)
-		return inset.latex(buf, os, runparams);
-	return 0;
+	return isBranchSelected(buf.params().branchlist()) ?
+		inset.latex(buf, os, runparams) : 0;
 }
 
 
 int InsetBranch::linuxdoc(Buffer const & buf, std::ostream & os,
 			  OutputParams const & runparams) const
 {
-	string const branch_sel = buf.params().branchlist().allSelected();
-	if (branch_sel.find(params_.branch, 0) != string::npos)
-		return inset.linuxdoc(buf, os, runparams);
-	return 0;
+	return isBranchSelected(buf.params().branchlist()) ?
+		inset.linuxdoc(buf, os, runparams) : 0;
 }
 
 
 int InsetBranch::docbook(Buffer const & buf, std::ostream & os,
 			 OutputParams const & runparams) const
 {
-	string const branch_sel = buf.params().branchlist().allSelected();
-	if (branch_sel.find(params_.branch, 0) != string::npos)
-		return inset.docbook(buf, os, runparams);
-	return 0;
+	return isBranchSelected(buf.params().branchlist()) ?
+		inset.docbook(buf, os, runparams) : 0;
 }
 
 
 int InsetBranch::plaintext(Buffer const & buf, std::ostream & os,
-		       OutputParams const & runparams) const
+			   OutputParams const & runparams) const
 {
-	string const branch_sel = buf.params().branchlist().allSelected();
-	if (branch_sel.find(params_.branch, 0) != string::npos) {
-		return inset.plaintext(buf, os, runparams);
-	}
-	return 0;
+	return isBranchSelected(buf.params().branchlist()) ?
+		inset.plaintext(buf, os, runparams): 0;
 }
 
 
@@ -207,12 +217,9 @@ InsetBranchMailer::InsetBranchMailer(InsetBranch & inset)
 {}
 
 
-string const InsetBranchMailer::inset2string(Buffer const & buf) const
+string const InsetBranchMailer::inset2string(Buffer const &) const
 {
-	InsetBranchParams params = inset_.params();
-	params.branchlist = buf.params().branchlist();
-	inset_.setParams(params);
-	return params2string(params);
+	return params2string(inset_.params());
 }
 
 
@@ -221,8 +228,6 @@ string const InsetBranchMailer::params2string(InsetBranchParams const & params)
 	ostringstream data;
 	data << name_ << ' ';
 	params.write(data);
-	// Add all_branches parameter to data:
-	data << params.branchlist.allBranches() << "\n";
 	return data.str();
 }
 
@@ -243,33 +248,24 @@ void InsetBranchMailer::string2params(string const & in,
 	if (name != name_)
 		return print_mailer_error("InsetBranchMailer", in, 1, name_);
 
+	// This is part of the inset proper that is usually swallowed
+	// by LyXText::readInset
+	string id;
+	lex >> id;
+	if (!lex || id != "Branch")
+		return print_mailer_error("InsetBranchMailer", in, 2, "Branch");
+
 	params.read(lex);
-	// Process all_branches here:
-	if (lex.isOK()) {
-		lex.next();
-		params.branchlist.add(lex.getString());
-	}
 }
 
 
 void InsetBranchParams::write(ostream & os) const
 {
-	os << "Branch" << " " << branch << "\n";
+	os << "Branch " << branch << '\n';
 }
 
 
 void InsetBranchParams::read(LyXLex & lex)
 {
-	if (lex.isOK()) {
-		lex.next();
-		string token = lex.getString();
-	}
-	if (lex.isOK()) {
-		lex.next();
-		string token = lex.getString();
-	}
-	if (lex.isOK()) {
-		lex.next();
-		branch = lex.getString();
-	}
+	lex >> branch;
 }
diff --git a/src/insets/insetbranch.h b/src/insets/insetbranch.h
index 6da8766643..f76465c2c9 100644
--- a/src/insets/insetbranch.h
+++ b/src/insets/insetbranch.h
@@ -14,18 +14,20 @@
 
 
 #include "insetcollapsable.h"
-#include "BranchList.h"
 
- struct InsetBranchParams {
+class BranchList;
+
+
+struct InsetBranchParams {
+	explicit InsetBranchParams(std::string const & b = std::string())
+		: branch(b) {}
 	///
 	void write(std::ostream & os) const;
 	///
 	void read(LyXLex & lex);
 	///
 	std::string branch;
-	/// Hack -- MV
-	BranchList branchlist;
- };
+};
 
 
 /** The Branch inset for alternative, conditional output.
@@ -34,7 +36,7 @@
 class InsetBranch : public InsetCollapsable {
 public:
 	///
-	InsetBranch(BufferParams const &, std::string const &);
+	InsetBranch(BufferParams const &, InsetBranchParams const &);
 	/// Copy constructor
 	InsetBranch(InsetBranch const &);
 	///
@@ -71,6 +73,12 @@ public:
 	InsetBranchParams const & params() const { return params_; }
 	///
 	void setParams(InsetBranchParams const & params) { params_ = params; }
+
+	/** \returns true if params_.branch is listed as 'selected' in
+	    \c branchlist.
+	 */
+	bool isBranchSelected(BranchList const & branchlist) const;
+
 protected:
 	///
 	virtual
diff --git a/src/lyxfunc.C b/src/lyxfunc.C
index ab10b7ab70..2ceb143dee 100644
--- a/src/lyxfunc.C
+++ b/src/lyxfunc.C
@@ -1404,7 +1404,8 @@ void LyXFunc::dispatch(FuncRequest const & func, bool verbose)
 				(lyx_name == lcolor.getLyXName(LColor::graphicsbg) &&
 				 x11_name != lcolor.getX11Name(LColor::graphicsbg));
 
-			if (!lcolor.setColor(lyx_name, x11_name)) {
+			LColor::color col = lcolor.getFromLyXName(lyx_name);
+			if (!lcolor.setColor(col, x11_name)) {
 				setErrorMessage(
 					bformat(_("Set-color \"%1$s\" failed "
 								"- color is undefined or "
diff --git a/src/lyxrc.C b/src/lyxrc.C
index 911c2a80cf..24c940267b 100644
--- a/src/lyxrc.C
+++ b/src/lyxrc.C
@@ -763,7 +763,8 @@ int LyXRC::read(string const & filename)
 				break;
 			}
 
-			if (!lcolor.setColor(lyx_name, x11_name)) {
+			LColor::color col = lcolor.getFromLyXName(lyx_name);
+			if (!lcolor.setColor(col, x11_name)) {
 				lyxerr << "Bad lyxrc set_color for "
 					<< lyx_name << endl;