[PATCH] BUG: 178658 Adding remote encoding support to Dolphin

[Peter Penz]

Hi Rahman,

Thanks for the patch, this was on my TODO list for several months already but I did not code a single line yet...

> On Tue, Mar 24, 2009 at 11:52 PM, Thiago Macieira <thiago at kde.org> wrote:
> >
> >>
> >> Patch looks good, but before you commit, can you fix your indentation
> >> problems? Please use the same indentation as the rest of Dolphin
> (Dolphin
> >> appears to be using 4-space indentation but your new file is doing
> >> 2-then-4
> >> and some braces are in weird positions).
> >
> >
> Here is a clean one. Properly indentated. So should I OK to commit

I think the patch looks good and should be committed. But I'd like to wait for David Faure's input too, as I also think we must prevent code duplication and Konqueror should just use the Dolphin KPart for this.

Regarding the patch: I did not check the DolphinRemoveEncoding class in detail (as you said it's based on Thiago's kremoteencodingplugin and Thiago already reviewed it). Some minor details I found (nitpicking stuff):

- dolphinui.rc: Could you please rename the new item to "change_remote_encoding" instead of "changeremoteencoding" to stay consistent with the other names?

- dolphinviewactionhandler.cpp: You include "dolphinnewmenuobserver.h", but have not used it (maybe a relict from the recent "Create New..." patch?).

- dolphinviewactionhandler.cpp: This very nitpicking, but I always try to keep the same order of the methods in the cpp file like in the h-file -> could you please move the new methods currentView() and actionCollection() below the same methods like in the h-file? Thanks ;-)

Best regards,
Peter

> 
> -- 
> Rahman Duran
> 
> Software Engineer
> Turkey
> 
> How many apples fell on Newton's head before he took the hint!