Code review comment for lp:~kartiksinghal/polly/patch-for-undo-redo-for-compose-box

Revision history for this message
Kartik Singhal (kartiksinghal) wrote :

On Sat, Jun 9, 2012 at 9:59 PM, Conscious User <email address hidden>wrote:

> Review: Needs Fixing
>
> Hi Kartik, thanks for the patch.
>

Hi, thanks for the review. :)

> Those are more "guideline fixes" than actual fixes, so it should be easy:
>
> 1) Please move the undobuffer module to the "external" folder and import
> it with "from polly.external import undobuffer". I do that for all modules
> copied from other sources.
>

Okay. Updating this.

> 2) It seems that you are associating undo with z and redo with y.
> Shouldn't it be ctrl+z and ctrl+y.
>

No, they are correctly associated to Ctrl+z and y respectively.
Gtk.keysyms.z maps to Ctrl+z itself.

> 3) The GNOME HIG guidelines, which I tend to follow, actually associate
> redo with shift+ctrl+z.
>

I had this doubt - what shortcut to use for redo while associating Ctrl+y.
I wasn't aware about the GNOME HIG, fixing.

> It is also needed to update the copyright file to reflect the new module,
> but you can leave that to me.
>

Sure.

--
Kartik
http://k4rtik.wordpress.com/

« Back to merge proposal