There are two changes in this patch:
1. Loading the history, the code used
while (!infile.eof()) {
getline(infile, ...);
...
}
That's simply wrong, because it will read one superfluous empty line at the
end. You need to check if reading succeeded instead of checking if the stream
already encountered EOF.
2. Writing back the history, the code tried to skip elements until it found
the current one, then write the rest and then finally the current command.
This avoids writing the beginning of the file, but firstly it doesn't work,
and secondly it is IMHO a completely unnecessary optimization because a) it
isn't called often enough and b) the file is <1kB here after years of use. I'm
using the most simple implementation and just write the whole history every
time, skipping all duplicates and finally append the current command.
There was one thing I'm not sure how the code was supposed to work and which
wasn't documented either. The question is how editing of previously entered
commands is reflected in the history. There are two approaches:
1. If I enter "xter" (if I pressed enter too early), going back in history and
then appending the "m", it replaces the "xter" with "xterm" in history.
2. If I enter "xter", going back in history and appending the "m" yields two
entries "xter" and "xterm".
I chose approach 1 in the patch. However, that means that if a user wants to
have two entries like "xterm sh" and "xterm bash", they can't achieve that by
just editing the history, they must enter them completely separately instead.
If you don't like that, just skip the code leading to the second "continue" in
the look that writes the history. ;)
Cheers!
Uli
Index: FbRun.cc
===================================================================
--- FbRun.cc (revision 34)
+++ FbRun.cc (revision 39)
@@ -137,43 +137,22 @@
hide(); // hide gui
// save command history to file
- if (text().size() != 0) { // no need to save empty command
+ if (!command.empty()) { // no need to save empty command
- // don't allow duplicates into the history file, first
- // look for a duplicate
- if (m_current_history_item < m_history.size()
- && text() == m_history[m_current_history_item]) {
- // m_current_history_item is the duplicate
- } else {
- m_current_history_item = 0;
- for (; m_current_history_item < m_history.size();
- ++m_current_history_item) {
- if (m_history[m_current_history_item] == text())
- break;
- }
- }
-
- fstream inoutfile(m_history_file.c_str(), ios::in|ios::out);
- if (inoutfile) {
- // now m_current_history_item points at the duplicate, or
- // at m_history.size() if no duplicate
- if (m_current_history_item != m_history.size()) {
- unsigned int i = 0;
- // read past history items before current
- for (; inoutfile.good() && i < m_current_history_item; i++)
- inoutfile.ignore(1, '\n');
-
- // write the history items that come after current
- for (i++; i < m_history.size(); i++)
- inoutfile<<m_history[i]<<endl;
-
- } else {
- // set put-pointer at end of file
- inoutfile.seekp(0, ios::end);
+ ofstream outfile(m_history_file.c_str());
+ if (outfile) {
+ for (unsigned i = 0; i != m_history.size(); ++i) {
+ // don't allow duplicates into the history file
+ if (m_history[i] == command)
+ continue;
+ // skip currently selected item, this will be appended but is
+ // not caught as duplicate when it was modified by the user
+ if (i == m_current_history_item)
+ continue;
+ outfile<<m_history[i]<<endl;
}
// append current command to the file
- inoutfile<<command<<endl;
-
+ outfile<<command<<endl;
} else
cerr<<"FbRun Warning: Can't write command history to file: "<<m_history_file<<endl;
}
@@ -197,8 +176,7 @@
m_history.clear();
// each line is a command
string line;
- while (!infile.eof()) {
- getline(infile, line);
+ while (getline(infile, line)) {
if (line.size()) // don't add empty lines
m_history.push_back(line);
}
