On 8/10/06, Chris MacRaild wrote:
On Thu, 2006-08-10 at 20:51 +1000, Edward d'Auvergne wrote:
> Thanks. I have a suggestion to simplify the state saving code and
> that is that the save_state function is called within the BaseError
> class. This means that save_state is only called from one place in
> the code by placing it within the __str__ function (which is called to
> get the error message). For example something like:
>
> class BaseError(Exception):
> def __str__(self):
> # Save the program state.
> if Debug:
> self.save_state()
>
> # The RelaxError message.
> return ("RelaxError: " + self.text + "\n")
There are a couple of reasons I didn't do it this way. One is that
__str__ is a 'special' function as far as Python is concerned, and it
exists for a specific purpose (same with all the double-underscore
functions). Although it's difficult to imagine how it might cause
problems in this case, I'm inclined to avoid messing with it on
principle.
Too late, I've already destroyed it ;) I replaced it so that it
returns the specific RelaxError message generated and stored in
'self.text'. I think the only problem with this special '__str__()'
function is that if it generates an error, you're in big trouble.
The second reason is that the way I've implimented it now
gives us greater control - perhaps there will be a set of Errors that we
dont want to save state. Or, as I've done with RelaxFault, we might want
to save state in all cases, not just if Debug (the logic here being that
RelaxFault always reflects a bug, and we want all the info we can about
it especially if it is difficult to reproduce). I appreciate that the if
Debug: self.save_state() gets repeditive, but I think its a price worth
paying in this case.
I didn't notice the different behaviour with RelaxFault, that
flexibility is useful as the user can then attach the saved state to
the bug report if needed. I hope you didn't get RSI from inserting
all those save state statements!
>
> def save_state(self):
> """Function for saving the program state"""
>
> now = time.localtime()
> file_name = "relax_state_%i%i%i_%i%i%i" % (now[3], now[4],
now[5],
> now[2], now[1], now[0])
> self._relax.interpreter._State.save(file_name)
>
> I would place the import statement at the top of the 'error.py' file
> with the other import statements (and the save_state function probably
> doesn't need the underscore out the front of it's name).
fine.
Just basic janitorial stuff. It might be worth swapping the dates
around so the year comes first. That way the saved states are ordered
by date in a directory listing. Also by using the string '%02i',
zeros will be placed in front of the single digit dates and times.
> Do you think
> it's worth having a separate command line flag for saving the state?
> I don't think so but it would result in finer control.
>
Not sure this is worth it, but fairly easy to impliment if anyone wants
to.
Unless there is a need for it, it's not worth the effort.
Edward
Re: r2533 - in /1.2: errors.py prompt/interpreter.py relax