Re: [Libreoffice] [GSOC][PATCH] Multiline inputbar

On 05/07/11 03:52, Anurag Jain wrote:
> Hello Noel,
>
> As you asked I'm sending a patch here with most of the changes
> incorporated. I've done total rework again and made a bit more
> modular, removed unnecessary things and now it look way better than
> what i was getting yesterday. Please have a look before you make
> changes on you end and if you think this is good enough to be pushed
> into Master please do so.
Sorry but it's a too late I have vacation Wed/Thur ( not sure when Kohei 
is back ) and the integration into master needs to be done by Thursday.  
I expected this patch much earlier ( by last Friday we agreed on IRC ) 
and and now time is too short. I already told you on IRC that I was 
integrating your code. It is now integrated on master and also for your 
convenience on the feature branch, see 
http://cgit.freedesktop.org/libreoffice/calc/commit/?id=971ff8c26b8d9559367a8b86fbf967327cbd70e0

Summary of the changes

a) Your code is now runtime switchable, by default the code runs as 
before, to enable your multiline input work then from libreoffice use 
Tools | Options | General | Enable experimental feature. No need to 
restart libreoffice and newly opened calc document will honour the setting
b) Made ScInputBarGroup and ScTextWnd substitutable by giving them a 
common base class, added the code to swap using the configure switch above
c) Since I never got the patch to restore the old ScTextWnd code or 
create the new class to capture the differences between your version of  
ScTextWnd & the original one I had to do this myself.
I notice in the file you sent yesterday and the patch today that you 
still made a copy of the complete class :-( When I integrated your code 
I just inherited from ScTextWnd and captured the changes that way. Also 
I didn't like ScMultiBar so I renamed it to ScMulitTextWnd, I am not 
thrilled with the new name but I am happier with it than the other name
d) for safety disabled the old accessibility related code when using the 
'new' inputline stuff

Note: it would be worth you checking that it runs as it should, I could 
have missed something ( although it seems to work as before )
>   I guess I'd be better if you make the
> runtime decision thing above this patch.
ideally it would have been better for you to do this but it seems I 
failed to be able to get you to understand what was required :-(
> As of now there are might be some printf's lying around so ignore them
> as of now.
> Awaiting your feedback on this.
I already explained to you on IRC that you will have to merge your 
changes into the new code base that resulted from me integrating your 
code. You at least need to do that for the feature branch. In the code 
integrated into master/feature-branch I haven't changed or cleaned up 
your code, just separated it from the core code so your modifications 
should be easy for you to port.
Note: please back up your work before doing a git pull in the feature 
branch so you can refer back to your latest changes
I will have a look at the patch as it stands and review it. Please try 
not to leave printf(s) in the code, use OSL_TRACE instead ( you need to 
do 'make dbglevel=2' to enable them). At least if an OSL_TRACE sneaks 
into master for example then normally it is compiled out and no 
unnecessary output will be sent to the terminal

Noel