Created attachment 20425 [details]
Making chainsaw remember filter expressions between restarts

In this patch, I have made the required changes to the LogPanel class in order
to make chainsaw remember filter expressions between restarts. I used the same
XML file which is currently used to remember LogPanel configurations to save
the filter expressions. And I added a button to clear the selected expression
on the combo box such that the cleared expression is no longer remembered by
Chainsaw.

Patch looks reasonable to me.  Possibly backward compatibility problem with the
new Vector being serialized.

Can we check if the returned Vector is null?  What if existing saved settings
don't have this, my guess is the Vector will be either be null (and therefore
the user will get an NPE on startup), or it will error during the xstream read
operation.

Isuru are you able to confirm this behaviour by deleting existing preferences,
use the non-patched version, saved settings, then apply patch and open
application.  Just want to confirm we do not introduce a bug to our existing users.

(In reply to comment #2)
> Patch looks reasonable to me.  Possibly backward compatibility problem with the
> new Vector being serialized.
> 
> Can we check if the returned Vector is null?  What if existing saved settings
> don't have this, my guess is the Vector will be either be null (and therefore
> the user will get an NPE on startup), or it will error during the xstream read
> operation.

Yes that's an issue. It returns an EOF exception when we try to load settings
when the vector is not in the XML file. This EOF exception is not handled in the
current implementation. I searched the javadoc for ObjectInputStream and
couldn't find a method to check whether the stream is over.

> 
> Isuru are you able to confirm this behaviour by deleting existing preferences,
> use the non-patched version, saved settings, then apply patch and open
> application.  Just want to confirm we do not introduce a bug to our existing
users.

As a solution for this issue,  we can save the vector before the last object
(Dimention object) and when reading, we can check the last element whether it is
an instance of Vector or Dimention. And read again only if it is a Vector
instance. This will avoid the possibility of occurring an EOF exception.
Will this be OK??

~Isuru

We have 2 options here I think:

1) Ignore the problem - if we do this, we should log the exception so that the
loading of the preferences at least appears inside Chainsaws own log panel. 
Currently there's a TODO to log it instead of printStackTrace().  

2) Introduce a version number in the serialization file.  During the
saveSettingsEvent, at the end of the current set of objects being saved, write a
version #.  This number can be read back and used to determine whether the next
object is likely to have been serialzide.  You could do something like:


..(previous deserializationstuff)
Vector v = null;
int versionNumber=0
try{
  int versionNumber = in.readInt();
  
}catch(Exception e){
  // no versionNumber found, might want to be specific about the Exception
  // caught here to make sure it's only End of Stream events.
}

// now we know where at a point that has introduced the versionNumber
if(versionNumber>0){
    v = in.readObject();
} 
if (versionNumber>1){
        // ... some other future serialized data structure
}

..etc

In the saveSettings event, we can start writing a version # at the end of the
current list, but before you add the Vector (and other objects if needed).

Does that make sense?

(In reply to comment #4)
> We have 2 options here I think:
> 
> 1) Ignore the problem - if we do this, we should log the exception so that the
> loading of the preferences at least appears inside Chainsaws own log panel. 
> Currently there's a TODO to log it instead of printStackTrace().  
> 
> 2) Introduce a version number in the serialization file.  During the
> saveSettingsEvent, at the end of the current set of objects being saved, write a
> version #.  This number can be read back and used to determine whether the next
> object is likely to have been serialzide.  You could do something like:
> 
> 
> ..(previous deserializationstuff)
> Vector v = null;
> int versionNumber=0
> try{
>   int versionNumber = in.readInt();
>   
> }catch(Exception e){
>   // no versionNumber found, might want to be specific about the Exception
>   // caught here to make sure it's only End of Stream events.
> }
> 
> // now we know where at a point that has introduced the versionNumber
> if(versionNumber>0){
>     v = in.readObject();
> } 
> if (versionNumber>1){
>         // ... some other future serialized data structure
> }
> 
> ..etc
> 
> In the saveSettings event, we can start writing a version # at the end of the
> current list, but before you add the Vector (and other objects if needed).
> 
> Does that make sense?

Yes I think your second suggestion is a reasonable way of tackling the problem.
If needed we'll be able to change this in the future when new data structures
will needed to be saved. I have tested it and it works fine. I'll attach the new
patch....

Created attachment 20483 [details]
Patch with a new way of remembering expressions

This attachment contains the implementation of a method to avoid a possible bug
in the previous patch...

Patch applied in revision 555100.    I changed the version number literal to a
constant and added your name to the author tags for the file.

Thanks Isuru.