Merge lp://qastaging/~dinko-metalac/ubuntu-keyboard/add-bosnian into lp://qastaging/ubuntu-keyboard

Can someone check this branch please? I really wish this to go into trunk :)

Hi Dinko!

 Thanks very much for helping expanding the keyboard layouts we support, it looks like you've accidentally forgotten to run "bzr add" on all your new files, so they haven't been added to the branch you submitted. Once the branch is updated I'll review again :)

Hi Michael,

Sorry for that. I've done that now, so the new code is in place.

Cheers

On Fri, Oct 24, 2014 at 4:37 PM, Michael Sheldon <
<email address hidden>> wrote:

> Review: Needs Fixing
>
> Hi Dinko!
>
> Thanks very much for helping expanding the keyboard layouts we support,
> it looks like you've accidentally forgotten to run "bzr add" on all your
> new files, so they haven't been added to the branch you submitted. Once the
> branch is updated I'll review again :)
> --
>
> https://code.launchpad.net/~dinko-metalac/ubuntu-keyboard/add-bosnian/+merge/238744
> You are the owner of lp:~dinko-metalac/ubuntu-keyboard/add-bosnian.
>

Looks good overall, there's one slight change needed to let the word engine know about your plugin (for prediction and spellchecking to work fully), you'll just need to add an entry for your plugin to WordEngine::onLanguageChanged in src/lib/logic/wordengine.cpp.

Thanks!

Hi,

Did that! Hope everything is ok now.

Cheers

On Fri, Oct 24, 2014 at 5:29 PM, Michael Sheldon <
<email address hidden>> wrote:

> Review: Needs Fixing
>
> Looks good overall, there's one slight change needed to let the word
> engine know about your plugin (for prediction and spellchecking to work
> fully), you'll just need to add an entry for your plugin to
> WordEngine::onLanguageChanged in src/lib/logic/wordengine.cpp.
>
> Thanks!
> --
>
> https://code.launchpad.net/~dinko-metalac/ubuntu-keyboard/add-bosnian/+merge/238744
> You are the owner of lp:~dinko-metalac/ubuntu-keyboard/add-bosnian.
>

Couple more things:

In ./plugins/bs/src/bosnianplugin.h it looks like you've missed a couple of class names that were copied over from the Serbian plugin (the class name itself and the constructor name).

Similarly lang_sr.path and lang_sr.files need to be changed to lang_bs.path and lang_bs.files in plugins/bs/qml/qml.pro

It looks like there isn't a hunspell-bs package in the Ubuntu repositories at the moment (nor a myspell or aspell Bosnian dictionary unfortunately), so you'll need to remove that dependency for now.

I've added a couple of comments to the diff at the relevant points to clarify the areas that need updating :)

Also, it looks like you're missing the "M" key from the URL and email layouts

Ok Michael,

I have a new code in the branch. Is it possible to use myspell-hr for
croatian language since bosnian and croatian are almost the same languages?
Are the changes in debian/control sufficient?

Cheers

On Fri, Oct 24, 2014 at 7:23 PM, Michael Sheldon <
<email address hidden>> wrote:

> Also, it looks like you're missing the "M" key from the URL and email
> layouts
> --
>
> https://code.launchpad.net/~dinko-metalac/ubuntu-keyboard/add-bosnian/+merge/238744
> You are the owner of lp:~dinko-metalac/ubuntu-keyboard/add-bosnian.
>

I've had a bit of a think about this and while it's not currently possible to use the Croatian dictionary it might be possible in the future. You can override the setLanguage function and force it to load "hr" data instead of "bs", however because we don't currently have a Croation layout plugin with prediction data this fails for now. Once we add a Croation plugin it should be possible to update the Bosnian plugin to make use of its data.

So for now could you remove the dependence on myspell-hr (since otherwise it'll just be taking up the user's space without doing anything), and we can add it back in the future. Once that's done this branch looks pretty much good to go :)

Ok, i will add croatian layout as well since they are the same actually. I
will create a separate branch for croatian.
On 28 Oct 2014 14:40, "Michael Sheldon" <email address hidden>
wrote:

> I've had a bit of a think about this and while it's not currently possible
> to use the Croatian dictionary it might be possible in the future. You can
> override the setLanguage function and force it to load "hr" data instead of
> "bs", however because we don't currently have a Croation layout plugin with
> prediction data this fails for now. Once we add a Croation plugin it should
> be possible to update the Bosnian plugin to make use of its data.
>
> So for now could you remove the dependence on myspell-hr (since otherwise
> it'll just be taking up the user's space without doing anything), and we
> can add it back in the future. Once that's done this branch looks pretty
> much good to go :)
> --
>
> https://code.launchpad.net/~dinko-metalac/ubuntu-keyboard/add-bosnian/+merge/238744
> You are the owner of lp:~dinko-metalac/ubuntu-keyboard/add-bosnian.
>

Awesome, thanks!

Could you leave the Bosnian database empty in this MR? Unfortunately the predictions aren't usable without a corresponding dictionary database so it'll just be taking up space on the device. Once this and the Croatian layout have landed I'll submit an additional MR to get this using the Croatian spelling and prediction databases (they have to be used together) as a temporary measure until we can find a Bosnian dictionary and get it packaged.

Did so.

PASSED: Continuous integration, rev:241
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/495/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/121
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/120
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/120/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/120

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/495/rebuild

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?

 * Yes

Did CI run pass? If not, please explain why.

 * Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

 * Yes (completed on submitter's behalf)