Avoid duplicates in minibuffer history

The removal of duplicates is done in LastCommandsSection::add and uses
the erase-remove idiom for performance.

Most of the patch is a cleanup of GuiCommandBuffer:

* remove history_ member, that was a copy of the session lastcommands
  vector. Use instead a wrapper history() around it and a addHistory
  wrapper for adding new entries.

* Make sure that there is only one place where commands are added to
  history. The code used to maintain a list for interactive editing,
  and a list for saving the session. They could be different in terms
  of leading/trailing spaces.

* [unrelated] remove command_ member, which is just a copy of
  LyXAction list of commmands. Use directly lyxaction instead.

src/Session.cpp

@@ -408,6 +408,11 @@ void LastCommandsSection::setNumberOfLastCommands(unsigned int no)
 
 void LastCommandsSection::add(std::string const & command)
 {
+	// remove traces of 'command' in history using the erase-remove idiom
+	//   https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom
+	lastcommands.erase(remove(lastcommands.begin(), lastcommands.end(), command),
+	                   lastcommands.end());
+	// add it at the end of the list.
 	lastcommands.push_back(command);
 }
 

src/Session.h

@@ -302,7 +302,7 @@ public:
 	void write(std::ostream & os) const override;
 
 	/// Return lastcommands container (vector)
-	LastCommands const getcommands() const { return lastcommands; }
+	LastCommands const & getcommands() const { return lastcommands; }
 
 	/** add command to lastcommands list
 	    @param command command to add

src/frontends/qt/GuiCommandBuffer.cpp

@@ -45,6 +45,15 @@ namespace frontend {
 
 namespace {
 
+// Simple wrapper around the LastCommand session stuff.
+
+// The whole last commands list
+vector<string> const & history() { return theSession().lastCommands().getcommands(); }
+
+// add command to the list (and remove duplicates)
+void addHistory(string const & cmd) { theSession().lastCommands().add(cmd); }
+
+
 class QTempListBox : public QListWidget {
 public:
 	QTempListBox() {
@@ -89,10 +98,6 @@ protected:
 GuiCommandBuffer::GuiCommandBuffer(GuiView * view)
 	: view_(view)
 {
-	for (auto const & name_code : lyxaction) {
-		commands_.push_back(name_code.first);
-	}
-
 	QPixmap qpup = getPixmap("images/", "up", "svgz,png");
 	QPixmap qpdown = getPixmap("images/", "down", "svgz,png");
 
@@ -141,26 +146,15 @@ GuiCommandBuffer::GuiCommandBuffer(GuiView * view)
 #endif
 	setFocusProxy(edit_);
 
-	LastCommandsSection::LastCommands last_commands
+	upPB->setEnabled(!history().empty());
-		= theSession().lastCommands().getcommands();
-	LastCommandsSection::LastCommands::const_iterator it
-		= last_commands.begin();
-	LastCommandsSection::LastCommands::const_iterator end
-		= last_commands.end();
 
-	upPB->setEnabled(it != end);
+	history_pos_ = history().end();
-
-	for(; it != end; ++it)
-		history_.push_back(*it);
-	history_pos_ = history_.end();
 }
 
 
 void GuiCommandBuffer::dispatch()
 {
 	std::string const cmd = fromqstr(edit_->text());
-	if (!cmd.empty())
-		theSession().lastCommands().add(cmd);
 	DispatchResult const & dr = dispatch(cmd);
 	if (!dr.error()) {
 		view_->setFocus();
@@ -174,13 +168,13 @@ void GuiCommandBuffer::dispatch()
 
 void GuiCommandBuffer::listHistoryUp()
 {
-	if (history_.size()==1) {
+	if (history().size() == 1) {
-		edit_->setText(toqstr(history_.back()));
+		edit_->setText(toqstr(history().back()));
 		upPB->setEnabled(false);
 		return;
 	}
 	QPoint const & pos = upPB->mapToGlobal(QPoint(0, 0));
-	showList(history_, pos, true);
+	showList(history(), pos, true);
 }
 
 
@@ -208,13 +202,11 @@ void GuiCommandBuffer::showList(vector<string> const & list,
 
 	// For some reason the scrollview's contents are larger
 	// than the number of actual items...
-	vector<string>::const_iterator cit = list.begin();
+	for (auto const & item : list) {
-	vector<string>::const_iterator end = list.end();
-	for (; cit != end; ++cit) {
 		if (reversed)
-			listBox->insertItem(0, toqstr(*cit));
+			listBox->insertItem(0, toqstr(item));
 		else
-			listBox->addItem(toqstr(*cit));
+			listBox->addItem(toqstr(item));
 	}
 
 	listBox->resize(listBox->sizeHint());
@@ -249,8 +241,8 @@ void GuiCommandBuffer::up()
 	if (!h.empty())
 		edit_->setText(toqstr(h));
 
-	upPB->setEnabled(history_pos_ != history_.begin());
+	upPB->setEnabled(history_pos_ != history().begin());
-	downPB->setEnabled(history_pos_ != history_.end());
+	downPB->setEnabled(history_pos_ != history().end());
 }
 
 
@@ -261,9 +253,9 @@ void GuiCommandBuffer::down()
 	if (!h.empty())
 		edit_->setText(toqstr(h));
 
-	downPB->setEnabled(!history_.empty()
+	downPB->setEnabled(!history().empty()
-			   && history_pos_ != history_.end() - 1);
+			   && history_pos_ != history().end() - 1);
-	upPB->setEnabled(history_pos_ != history_.begin());
+	upPB->setEnabled(history_pos_ != history().begin());
 }
 
 
@@ -278,7 +270,7 @@ void GuiCommandBuffer::hideParent()
 
 string const GuiCommandBuffer::historyUp()
 {
-	if (history_pos_ == history_.begin())
+	if (history_pos_ == history().begin())
 		return string();
 
 	return *(--history_pos_);
@@ -287,9 +279,9 @@ string const GuiCommandBuffer::historyUp()
 
 string const GuiCommandBuffer::historyDown()
 {
-	if (history_pos_ == history_.end())
+	if (history_pos_ == history().end())
 		return string();
-	if (history_pos_ + 1 == history_.end())
+	if (history_pos_ + 1 == history().end())
 		return string();
 
 	return *(++history_pos_);
@@ -300,9 +292,9 @@ vector<string> const
 GuiCommandBuffer::completions(string const & prefix, string & new_prefix)
 {
 	vector<string> comp;
-	for (auto const & cmd : commands_) {
+	for (auto const & act : lyxaction) {
-		if (prefixIs(cmd, prefix))
+		if (prefixIs(act.first, prefix))
-			comp.push_back(cmd);
+			comp.push_back(act.first);
 	}
 
 	if (comp.empty()) {
@@ -345,10 +337,10 @@ DispatchResult const & GuiCommandBuffer::dispatch(string const & str)
 		return empty_dr;
 	}
 
-	history_.push_back(trim(str));
+	addHistory(trim(str));
-	history_pos_ = history_.end();
+	history_pos_ = history().end();
-	upPB->setEnabled(history_pos_ != history_.begin());
+	upPB->setEnabled(history_pos_ != history().begin());
-	downPB->setEnabled(history_pos_ != history_.end());
+	downPB->setEnabled(history_pos_ != history().end());
 	FuncRequest func = lyxaction.lookupFunc(str);
 	func.setOrigin(FuncRequest::COMMANDBUFFER);
 	return lyx::dispatch(func);

src/frontends/qt/GuiCommandBuffer.h

@@ -77,12 +77,6 @@ private:
 	/// dispatch a command
 	DispatchResult const & dispatch(std::string const & str);
 
-	/// available command names
-	std::vector<std::string> commands_;
-
-	/// command history
-	std::vector<std::string> history_;
-
 	/// current position in command history
 	std::vector<std::string>::const_iterator history_pos_;