[search] Replace GtkTextView with GtkSourceview

Bug #1349838 reported by Sagar Ghuge
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Xpad
Fix Released
Low
Siergiej Riaguzow

Bug Description

GtkSourceView is able to provide the undo/redo functionality and also searching the text makes simple as API provides the search funtionality.

Related branches

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

If we replace GtkTextView with GtkSourceView, GtkTextBuffer needs to be replaced with GtkSourceBuffer.

Source: https://developer.gnome.org/gtksourceview/stable/GtkSourceView.html

Revision history for this message
Sagar Ghuge (ghugesss) wrote :

Replaced the TextView/Buffer with the GtkSourceView/Buffer.

Once this will gets approved will start working on Search utility.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote : Re: [Bug 1349838] Re: Replace GtkTextView with GtkSourceview

Hi Sagar,

I have read through the code and what I have read it looks good and
promissing.
This is a very good first step for the preparation to get new functionality
working, such as the search.

For now I have only noticed one small thing to improve which are multiple
additions of parenthesis, which IMO are not necessary and therefore make
the code reading more complicated.

- g_signal_connect_swapped (gtk_text_view_get_buffer (GTK_TEXT_VIEW
(pad->priv->textview)), "notify::has-selection", G_CALLBACK
(xpad_pad_notify_has_selection), pad);

+ g_signal_connect_swapped ((gtk_text_view_get_buffer (GTK_TEXT_VIEW
(pad->priv->textview))), "notify::has-selection", G_CALLBACK
(xpad_pad_notify_has_selection), pad);

If there is a need for these parenthesis, please explain it to me.
If not can you remove these?

After that I will test the code and if everything is still working, I
will integrate it into the main branch.

Altogether: awesome work!

On 29 September 2014 15:27, Sagar Ghuge <email address hidden> wrote:

> Replaced the TextView/Buffer with the GtkSourceView/Buffer.
>
> Once this will gets approved will start working on Search utility.
>
> ** Patch added: "replace_textview_buffer_with_sourceview_buffer.patch"
>
> https://bugs.launchpad.net/xpad/+bug/1349838/+attachment/4219128/+files/replace_textview_buffer_with_sourceview_buffer.patch
>
> --
> You received this bug notification because you are subscribed to Xpad.
> Matching subscriptions: Arthur Borsboom
> https://bugs.launchpad.net/bugs/1349838
>
> Title:
> Replace GtkTextView with GtkSourceview
>
> Status in Xpad:
> New
>
> Bug description:
> GtkSourceView is able to provide the undo/redo functionality and also
> searching the text makes simple as API provides the search
> funtionality.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/xpad/+bug/1349838/+subscriptions
>

--
Arthur Borsboom
Lieven de Keystraat 77
3067 KG, Rotterdam
The Netherlands
Mob: +31629089953
Email: <email address hidden>
Skype: Arthur Borsboom, The Hague, The Netherlands

[image: View Arthur's LinkedIn profile]
<http://uk.linkedin.com/in/arthurborsboom>

Sagar Ghuge (ghugesss)
Changed in xpad:
assignee: nobody → Sagar Ghuge (ghugesss)
Changed in xpad:
milestone: none → 4.4
importance: Undecided → Low
Revision history for this message
Arthur Borsboom (arthurborsboom) wrote : Re: Replace GtkTextView with GtkSourceview

Hi Sagar,

I have tested the code and it seem to work on my side.
The memory usage increased a little bit, I guess due to the added library.
Let's see if we can reduce the memory usage elsewhere to make up for this.

I have removed two sets of parenthesis in two different lines.
I have updated the Changelog.

The above patch is merged into the main branch.

Changed in xpad:
status: New → Fix Committed
Changed in xpad:
status: Fix Committed → Fix Released
Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I've first implemented xpad-undo.h and xpad-undo.c and as I was much younger and more stupid there are most probably bugs. THere. Anyway i gave at leas a chance to bring back your prevous notes once you've deleted them.

Next thing which bothered me was lack of search in xpad which (for big nottes like "work.inf" make them only half useful). Have you also added search?

I'm seeing my most probably buggy from xpad-undo.c functions still being called: https://launchpadlibrarian.net/186046474/replace_textview_buffer_with_sourceview_buffer.patch. I feel bad about this, Can I try retest that and cleaning up>?

PS Alsp I do need search, do I understand that with you changes it comes for free and there wili be a button/menu in current node (like "Find")?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

PS Is search (Ctr-F) working now? In Arch package for example now. Have you made changes to loginc on xpad_undo functons? I believed I was able to break it with some corner cases. This was long ago unfortunately.

For example:
1. Type 12345
2. Hit Enter
3. Press b
4. Go back o first line and type xxx in the end so it looks like xxx.
5. Add 2 to 12345xxx so that it is 12345xx2x

Not repeatedly press Undo/Redo. At one (I'm not yet sure which) moment instead of working correct;u 12345 after disspearing won't appear back again with redo.

Should I find exact steps to reproduce/subit a bug/assign to myself? I'm feeling bad about this bug.

And what about search? Is is planned?

I remember creating a bug about it but I don't know what's the final state of this: "https://bugs.launchpad.net/xpad/+bug/452226"

Don't what to introduce bugs in that, would better make sure there are no more bugs in undo/redo

What's you opinion? Can I clean up myself a little (it should concern xpad-undo.h and xpad-undo.c

THank you!

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Hi Siergiej,

Welcome back!
You do not need to worry about making mistakes. We all do, continuously. :)

I am happy to hear that you are willing to fix some long lasting bugs, although I haven't noticed them. If you find bugs, please create a bug report, or (as you noticed) continue with an existing one. The bug report is used as a reference in the release notes.

I can assign you to the bug report and you can start working on it. You can either provide a pull request or a patch. I will then apply it locally, execute a review and few tests, and if all okay push it into the next version.

You want to start fixing the undo bug you had in mind?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

> You want to start fixing the undo bug you had in mind?
Yes, I do. Hope to be done by the end of month (I also have a job to do).

Will do as you told me, i.e.: will find some cases of "steps to reproduce" and create a bug and assigned myself to it. I believe I may be still in developers groups or smth so there will be no problem with doing so.

As for search, I trust you will add one as you are assigned to https://bugs.launchpad.net/xpad/+bug/452226/.

BTW my last work was in Windows and I totally forgot about xpad. Now I must use Linux and I was very disappointed and surprised how unusable new stuff (like Ubuntu new Window Manager, don't even know the name, Unity or Gnome3 maybe?) is. I had to learn how to customize Awesome WM and learned to use it instead.

And with it I also started to again use xpad.

IMVHO opinion it proves that despite of the fact that it is written in C in OOP manner (which leads to an over-bloated code) it is still a good S/W: it does one thing and does it right.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

That sounds cool!

No worries about the time path... I am absolutely in the relax mode too. :)

While looking longer at this bug report, I noticed that the textview has never been replaced with the sourceview, although this bug report says 'Fix released'. I do see a couple of references to sourceview, but textview is still used everywhere.

I will change this bug report back from 'fix released' to 'confirmed' and assign it to your name.

If you are stuck somewhere, let me know.

Changed in xpad:
status: Fix Released → Confirmed
Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

The old patch of Sagar might be a nice help, if you need inspiration. :)

Changed in xpad:
assignee: Sagar Ghuge (ghugesss) → Siergiej Riaguzow (riaguzov)
summary: - Replace GtkTextView with GtkSourceview
+ [search] Replace GtkTextView with GtkSourceview
Changed in xpad:
milestone: 4.4 → 5.3.0
Changed in xpad:
milestone: 5.3.0 → 5.4.0
Changed in xpad:
milestone: 5.4.0 → none
Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Hi Arthur,

Can you please review and merge this:
http://bazaar.launchpad.net/~riaguzov/xpad/search/revision/1002

This is just a beginning. You can find a TODO in the commit message. It appears that GtkSourceView WAS actually added by Sagar he has just left XpadTextBuffer which is just a wrapper around it.

This commit only removes my hand-written xpad-undo.[c.h] which is good! GtkSourceBuffer is capable of that so I've removed my old implementation and reused the GtkSourceBuffer one

The TODOs are as follows:
- Rename XpadTextBuffer to XpadSourceBuffer or remove it alltogether (what is simpler)
- Implement search as if is available in GtkSourceBuffer (easy)
- Follow some guids and clang suggestions to clean up code and remove duplicates from GTK3 (as xpad now depends on gtksourceview (well maybe I'm wrong about following this: https://developer.gnome.org/gtk3/stable/gtk-migrating-2-to-3.html as gtksourceview is also availble on GTK2

Anyway I was using gtksourceview3 and GTK3 headers. Hope nothing has changed that much.

I will continue on my branch to add search and make this cleanup with misleading XpadTextBuffer name which is just a wrapper aroung GtkSourceVIew, but maybe you can try compiling this one and merging it as it is alone is worth if only because it removed my novice xpad-undo stuff

Thanks!

Changed in xpad:
status: Confirmed → In Progress
Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

PS Arthur, BTW, can you clarify if xpad should be buildable with both GTK2 and GTK3? or only GTK3?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Arhur, it's a pity launchpad doesn't seem to allow to edit comments.

Anyway, looking through the code, XpadTextView and XpadTextBuffer have parents GtkSourceView and GtkSourceBuffer respectively and they use casts inside of them and they do have some code so Sagar seems to made his job just fine.

It's just that he has left the names misleading. IMHO what should've been done is just renaming:

XpatTextView - XpadSourceView
XpadTextBuffer - XpadSourceBuffer
(instead of removing them alltogether since they do have some code inside)

As removing XpadUndo (which I ask you to put accept and merge) appeared to be easy and everything worked (by using undo/redo of GtkSourceBuffer) I think that implementing search (the bug that is preventing myself from using xpad) should be relatively easy too.

I've just removed XpadUndo now. I can create a patch for this change instead of pointing to a commit in a branch if this is easier for you. If you will accept it I can then implement search since this seems to be major.

And after search is accepted I can then created a separate branch for renaming the "classes" and cleanups (as per clang, and migrating guides) since these are minor. If you will confirm xpad should be buildable with GTK2 I will do that while making my changes to ensure everything is working.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Hi Siergiej,

Thanks for taking the time to look through the code and improve it.
It is good to have a fresh set of eyes helping out!

I have skimmed through your merge request and it looks good, since it only contains the removal of Xpad undo. The to do list is a nice aid how to move forward and I like it.

Yesterday I have arrived in a new city to live, so I need a bit of time to unpack and get my bearings. After that I will give your merge request a test run and if the basic things seem to work, I will merge it back into the main branch.

I will get back to you within a couple of days.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

And as far as I know, Xpad works only with GTK3, since it uses many features which are GTK3 only.

So for simplicity I suggest to stick with GTK3 only, unless you have reasons to support GTK2.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

OK, so let's me clean it up just a tiny little bit (it has a whitespace change in xpad-text.c for example and I've seen a little bit different way of "upcasting" in OOP in C in xpad code, so I'll conform to that. Also I forgot to add replacements for freeze/thaw and they are really needed since otherwise Ctrl-Z right after opening a pad with a text will remove the whole text.

Just fixed all that and did a git push --overwrite. The link is the same: http://bazaar.launchpad.net/~riaguzov/xpad/search/revision/1002

After this is pushed I will do the search thing.

Thanks!

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

grrr.. launchpad - give me the edit comment button!

git push --overwrite # I meant "bzr push --overwrite"

After this is pushed # I meant "after that is merged to trunk"

And I'm sorry for asking merging commit by commit but I think search itself would take just 1 commit and not much time to do also since Sagar made a good work for replacing GtkText... to GtkSource... (he just left the naming of "inherited" "classes" like XpadText... which is misleading, should've beed XpadSource...). I'm just afraid to screw something up since I forgot bazaar. So I want this merged first, then search (in one commit also) merged second.

And only after that I will create a separate cleanup branch called "cleanup" or something and in that branch I won't bother you with each commit, instead will clean whatever, rename whatever and so on because this will not affecting functionality and can wait.

Changed in xpad:
status: In Progress → New
status: New → In Progress
Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Hi Siergiej,

I have downloaded your revision 1002 as a diff file and applied it to the main branch.

Undo and redo seem to be working, but I have been able to generate a GTK warning in the following cases.

Case 1
-----------------------
Create new / empty pad
Press CTRL-Z

Case 2
----------------------
Create a pad with text
Quit Xpad
Start Xpad
Press CTRL-Z

This generates the following error message.

GtkSourceView-CRITICAL: gtk_source_buffer_real_undo: assertion 'gtk_source_undo_manager_can_undo (buffer->priv->undo_manager)' failed

You think you can prevent/fix this warning of being generated?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I'll try today, thanks for noticing.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Arthur can you please try again with the same link http://bazaar.launchpad.net/~riaguzov/xpad/search/revision/1002? I've just added if's around undo/redo to avoid the warning.

PS The branch should have been called undo actually, but that's just one commit, maybe you can cherry-pick that or something?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I've actually changed it (for the last time I hope to make even less modifications (moved freezing undo back to xpad-text-view, this way it's just one file modified and two removed. Same link http://bazaar.launchpad.net/~riaguzov/xpad/search/revision/1002

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

I have applied the diff of 1002, but it generates build warnings.

xpad-text-buffer.c: In function ‘xpad_text_buffer_freeze_undo’:
xpad-text-buffer.c:422:47: warning: passing argument 1 of ‘gtk_source_buffer_begin_not_undoable_action’ from incompatible pointer type [-Wincompatible-pointer-types]
  gtk_source_buffer_begin_not_undoable_action (buffer);
                                               ^~~~~~
In file included from /usr/include/gtksourceview-3.0/gtksourceview/gtksource.h:27,
                 from xpad-text-buffer.c:26:
/usr/include/gtksourceview-3.0/gtksourceview/gtksourcebuffer.h:189:72: note: expected ‘GtkSourceBuffer *’ {aka ‘struct _GtkSourceBuffer *’} but argument is of type ‘XpadTextBuffer *’ {aka ‘struct XpadTextBuffer *’}
 void gtk_source_buffer_begin_not_undoable_action (GtkSourceBuffer *buffer);
                                                       ~~~~~~~~~~~~~~~~~^~~~~~
xpad-text-buffer.c: In function ‘xpad_text_buffer_thaw_undo’:
xpad-text-buffer.c:428:45: warning: passing argument 1 of ‘gtk_source_buffer_end_not_undoable_action’ from incompatible pointer type [-Wincompatible-pointer-types]
  gtk_source_buffer_end_not_undoable_action (buffer);
                                             ^~~~~~
In file included from /usr/include/gtksourceview-3.0/gtksourceview/gtksource.h:27,
                 from xpad-text-buffer.c:26:
/usr/include/gtksourceview-3.0/gtksourceview/gtksourcebuffer.h:192:70: note: expected ‘GtkSourceBuffer *’ {aka ‘struct _GtkSourceBuffer *’} but argument is of type ‘XpadTextBuffer *’ {aka ‘struct XpadTextBuffer *’}
 void gtk_source_buffer_end_not_undoable_action (GtkSourceBuffer *buffer);

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

F... It should've been parent. Please try 1 last time: http://bazaar.launchpad.net/~riaguzov/xpad/search/revision/1002. Must be good now. Sorry for being sloppy, I have to get used to bzr back again.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Please just remove this:

xpad-text-view.c:
424
:

I cannot bzr push :parent --overwrite since you probably are trying to get it:

Unable to obtain lock held by <email address hidden> on taotie (process #26397), acquired 3 minutes, 18 seconds ago.
See "bzr help break-lock" for more.
bzr: ERROR: Could not acquire lock "(remote lock)": bzr+ssh://bazaar.launchpad.net/~riaguzov/xpad/search/

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Fixed. Was able to push. No ':' anymore. Must build. Must work.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

I guess there is cast missing.
Or you might want to use the already defined and unused 'parent' variable.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

The revision looks good. I have downloaded the diff and tested it.

I am also not that handy with bazaar.
Somehow I should be able to pull your revision into the main branch.

For now I have applied it as a patch and pushed it to the main branch.

Is that okay for you?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Absolutely.

Actually this is better then merging in this case. The branch was called wrong and it had just one commit. Does bzr support fast-forward merge? Like rebase and then apply the commits like there was no branch at all?

If merging/rebasing/xxx is tiresome with bzr I'll continue with "gerrit-like-mode", I will create branches with just one commit for each task, bug, whatever so that it can be applied as a patch and committed with commit message copy-pasted.

Anyway, I'll switch to these:
https://bugs.launchpad.net/xpad/trunk/+bug/452226
https://bugs.launchpad.net/xpad/+bug/1811576

and then will come back to this one to cleanup (at least I think XpadText... should now be called XpadSource... as they "derive" from GtkSource...).

Thanks and sorry for so many generated noise around this.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

No worries for the noise.

I have been using bzr as a single man project (for 7 years by now?) and I have to get used to that too. I am happy somebody is helping out. So all is good!

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I've just noticed that I removed part of the functionality. Now it is impossible to undo making a text bold for example. I believe I have to change the way tags are applied. Will try to finish today

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Okay, clear.

Could you also verify the CTRL-Z key assignment?
Is that still necessary, or is it automatically added by the the GtkSourceView?

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

You mean this?

   g_signal_connect_swapped (pad->priv->toolbar, "activate-undo", G_CALLBACK (xpad_pad_undo), pad);

It is not needed animore. This is though (otherwise there will be no undo element):

   MENU_ADD (_("_Undo"), "edit-undo", GDK_KEY_Z, GDK_CONTROL_MASK, xpad_pad_undo);

PS Looks like to bring back undo/redo of making bold or strikethrough one has to write GtkSourceUndoManager https://developer.gnome.org/gtksourceview/stable/GtkSourceUndoManager.html#gtk-source-undo-manager-can-undo-changed and this brings back to where it started. I'll try to find a way to do it without implementing Undo back again.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I've greped through gtksourceview code and the default undo manager has only this:

    typedef enum _ActionType
    {
     ACTION_TYPE_INSERT,
     ACTION_TYPE_DELETE
    } ActionType;

So it is not available of dealing with tags as XpadUndo was and it is not that easy to extend it (apart from copy-paste). It does not give access to at least count of Actions in it's queue and all the functions are in .c file and are static. There is a _GtkSourceUndoManager interface but it only declares 8 function pointers like undo/redo and cannot be used and more fine-grained level.

So own GtkSourceUndoManager would have to be implemented, which would be a copy-paste of the one in GtkSourceView with addition to new action types. And that it exactly what XpadUndo was.

I propose for now to live with that, since GtkSourceView will give a chance to implement search, XpadUndo was able to undo bolding and such, but was buggy. And xpad is not a Word Processor so not being able to undo bolding is not a big issue (at least with ctrl-z it is possible to bring back deleted important content, at that is more important.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

PS Let's come back to this after search is done (if really needed I can create a XpadSourceUndoManager out of the GtkSourceUndoManagerDefault (copy-paste mostly) with addition of undoing bolding and such, but there should be an information if it is that much needed).

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

This is actually done, but to close is the following should be made:

- cleanups with names (XpadText* -> XpadSource*)
- cleanup with returning pointer to either GtkWidget or correct type in different xpad classes. In GTK3 most API _new() calls return GtkWidget* but this seams to be not the situation for libraries (if you will see gtksourceview library it returns XXX* xxx_new()). So why not do the same in xpad (i.e. return xxx* in xxx_new() and not GtkWidget*
- Some struct XXXPrivate have pointers to GtkWidget* and some have pointers to derived types. Independent of whether xpad's xxx_new() should return GtkWidget* or xxx* the struct XXXPrivate should have xxx* and not GtkWidget* inside for readability
- Last but most important: relying on GtkSourceBuffer from gtksourceview library for implementing undo/redo REMOVED possibility to undo/redo tags (like bold/underscore/etc). To bring it back XpadSourceUndoManager should be introduced and it will be a step back (XpadUndo was the same thing). I think it should be possible to reuse XpadSourceUndoManagerDefault (by deriving it) to add ACTION_TYPE_ADD_TAG, ACTION_TYPE_REMOVE_TAG to:

    typedef enum _ActionType
    {
        ACTION_TYPE_INSERT,
        ACTION_TYPE_DELETE
    } ActionType;

and with minimal copy paste introduce own XpadSourceUndoManager reusing as much code as possible from XpadSourceUndoManagerDefault which only handles insertions and deletions text for undo/redo (is not aware of tags at all). If this approach won't work maybe some workarounds can be added to still use XpadSourceUndoManagerDefault but above it also handle tags depending on the count of Actions inside of the manager.

At lest the last issue should be fixed since it is a step back from what XpadUndo was capable of before closing this task.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Considering conventions:

From the answers in the Russian site linux.org.ru I've found out that maybe GTK guys have created their conventions by statistics. The most common scenarion when creating a GTK widget is to put it to container and add callbacks and such and forget about. So you need to upcast in GtkWidget* to do all that. As there are no upcasts in C they just decided to return GtkWidget* for every derived from GtkWidget type i type_new().

On the other hand what is stored in Private is also a question of convenience. If most things you do with widgets in you *Private is calling functions which accept GtkWidget or something in the middle of this then is make sense to store them as GtkWidget*. If mostly you use them with functions like concre_type_do_smth(...) then it is better to store them as ConcreteType.

From my observation when *Private if full of GtkWidgets it is very hard to understand the layout of the complex widget. So I propose to follow one of this two solutions:

First solution:

1. As all GTK *xxx_new() functions which return anything deriving from GtkWidget* return GtkWidget* and not Xxx* we should do the same
2. To increase readability we should store pointers to concrete types in *Private like Xxx* so that layout if obvious.

Second solution

1. As xpad is not a library we can as well return Derived* from deriver_new() even if Derived inherits from GtkWidget
2. Same as above

I think any of this solution is OK as long as we are consistent and don't store part of things derived from GtkWidget* as GtkWidget* in *Private and part as DerivedFromGktWidget*.

Am I really alone with that code style issue? No one cares, except me?

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote : Re: [Bug 1349838] Re: [search] Replace GtkTextView with GtkSourceview

SR> Am I really alone with that code style issue? No one cares, except me?

That I don't know. :)

I can't really make the better choice of your two or even other suggestions.
However, I am a fan of code readability, which makes maintenance much
easier.

So, feel free to choose any option, which improves readability.

On Thu, 31 Jan 2019 at 20:10, Siergiej Riaguzow <email address hidden>
wrote:

> Considering conventions:
>
> >From the answers in the Russian site linux.org.ru I've found out that
> maybe GTK guys have created their conventions by statistics. The most
> common scenarion when creating a GTK widget is to put it to container
> and add callbacks and such and forget about. So you need to upcast in
> GtkWidget* to do all that. As there are no upcasts in C they just
> decided to return GtkWidget* for every derived from GtkWidget type i
> type_new().
>
> On the other hand what is stored in Private is also a question of
> convenience. If most things you do with widgets in you *Private is
> calling functions which accept GtkWidget or something in the middle of
> this then is make sense to store them as GtkWidget*. If mostly you use
> them with functions like concre_type_do_smth(...) then it is better to
> store them as ConcreteType.
>
> >From my observation when *Private if full of GtkWidgets it is very hard
> to understand the layout of the complex widget. So I propose to follow
> one of this two solutions:
>
> First solution:
>
> 1. As all GTK *xxx_new() functions which return anything deriving from
> GtkWidget* return GtkWidget* and not Xxx* we should do the same
> 2. To increase readability we should store pointers to concrete types in
> *Private like Xxx* so that layout if obvious.
>
> Second solution
>
> 1. As xpad is not a library we can as well return Derived* from
> deriver_new() even if Derived inherits from GtkWidget
> 2. Same as above
>
> I think any of this solution is OK as long as we are consistent and
> don't store part of things derived from GtkWidget* as GtkWidget* in
> *Private and part as DerivedFromGktWidget*.
>
> Am I really alone with that code style issue? No one cares, except me?
>
> --
> You received this bug notification because you are subscribed to Xpad.
> Matching subscriptions: Arthur Borsboom
> https://bugs.launchpad.net/bugs/1349838
>
> Title:
> [search] Replace GtkTextView with GtkSourceview
>
> Status in Xpad:
> In Progress
>
> Bug description:
> GtkSourceView is able to provide the undo/redo functionality and also
> searching the text makes simple as API provides the search
> funtionality.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/xpad/+bug/1349838/+subscriptions
>

--
Arthur Borsboom
Mob: +31629089953
Email: <email address hidden>
[image: View Arthur's LinkedIn profile]
<http://uk.linkedin.com/in/arthurborsboom>

Changed in xpad:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.