#636632 fbrun history is broken

Package:
fluxbox
Source:
fluxbox
Description:
Highly configurable and low resource X11 Window manager
Submitter:
Ulrich Eckhardt
Date:
2015-06-08 18:33:03 UTC
Severity:
normal
#636632#5
Date:
2011-08-04 18:42:13 UTC
From:
To:
The history management of fbrun is broken. The most obvious symptom is that
after starting something with it, the next time fbrun is started the cursor up
button doesn't reproduce that command. While one could argue that this is
still subjective, there is another thing that isn't.

The part that is more of a proof is the smashed content of the history file.
For that try the following:
0. remove ~/.fluxbox/fbrun_history (or back it up)
1. run fbrun and enter "xterm" as command
2. run fbrun and enter "sed" as command
3. run fbrun and enter "grep" as command
4. run fbrun and enter "xterm" as command
5. run fbrun and enter "xterm" as command

Don't use the history for entering xterm the second and third time, type them
manually! The contents of fbrun_history on my machine are now:

| sexterm
|
| xterm

I've been looking around the sources a bit, and I'll try to provide a patch. I
have already found one error and one thing that at least makes no sense to me.


Cheers!

Uli

#636632#10
Date:
2011-08-04 20:01:40 UTC
From:
To:
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

#636632#15
Date:
2012-02-15 22:51:16 UTC
From:
To:
...in the new 1.3.2-2, verified on one PowerPC system and another x86 system.
I'm wondering, is there a problem with either the description of the problem
or the patch?

Cheers!

Uli

#636632#22
Date:
2012-02-15 23:53:31 UTC
From:
To:
That's a good question. Bugs have been falling off my radar.

Just sent a FW to the upstream bug tracker.

I've been trying to avoid applying patches locally in Debian as a
general rule, so that's why you've not seen it here yet. If It's in the
upstream git repo, I'd be happy to include it, though.

HTH, thanks for the patch!
Paul