Angus Leeming <[EMAIL PROTECTED]> writes:

| Lars Gullik Bjønnes wrote:
>
>> This patch introduces the DispatchResult::dispatched() function and
>> removes DISPATCHED, UNDISPATCHED from dispatch_result_t, and changes
>> DISPATCHED_NOUPDATE to NOUPDATE in dispatch_reuslt_t,
>
| The comment in dispatchresult.h still refers to DISPATCHED_NOUPDATE.

I almost changed it, and I almost deleted it, so I decided to leave it
for the time being.

| Do you still need the default constructor for DispatchResult?

probably not
later patch

| This is a bit ugly, don't you think?
| cursor.dispatch(FuncRequest(func, view())).dispatched()

| As dispatched() now returns a bool, why not rename it as success()? 
| The code above is then very elegant.

Well it depends... remember that we are executing a FuncRequest, if it
were dispatched or not does not _really_ mean that is was successful
or unsuccessful.

f.ex. if cursor at the last position in the doc and then LFUN_RIGHT.
Then LFUN was dispatched alright, but it was not successful.

a better name could be "handled" perhaps.

| Ditto, here
| result = inset->dispatch(FuncRequest(view(), action, argument));
| if (result.dispatched()) {
>
| If you were to change 'result' to 'dispatch', the code would read
| if (dispatch.success()) {

This would then be:

     if (result.handled()) {

(but then the variable should probably change name again)

I have some notion to rename DispatchResult to FuncResult as well...
(but this should really only be a member of DispatchResult...)

| (Don't get me wrong; I think your code is a change for the good. I'm 
| just commenting on it since you posted it.)
>
| Staying on the subject of names:
| if (result.val() == FINISHED) {
>
| why not rename val() as status()?
| if (dispatch.status() == FINISHED) {

later patch.

-- 
        Lgb

Reply via email to