After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 163014 - XML helper plugin for gedit
XML helper plugin for gedit
Status: RESOLVED WONTFIX
Product: gedit-plugins
Classification: Other
Component: General
2.8.x
Other All
: Normal enhancement
: ---
Assigned To: Gedit maintainers
Gedit maintainers
: 154080 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-01-05 13:36 UTC by Bert Vermeulen
Modified: 2019-03-23 20:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
XML helper plugin (3.19 KB, application/x-compressed-tar)
2005-01-05 13:38 UTC, Bert Vermeulen
Details
corrected xmlhelper (8.94 KB, text/plain)
2005-01-08 08:06 UTC, Bert Vermeulen
Details
new xmlhelp.c (9.50 KB, text/plain)
2005-01-09 02:25 UTC, Bert Vermeulen
Details
XML helper plugin (20.00 KB, application/x-tar)
2005-01-10 00:37 UTC, Bert Vermeulen
Details

Description Bert Vermeulen 2005-01-05 13:36:13 UTC
This plugin includes two functions:

- close innermost open XML tag (bound to ctrl-m)

- open new tag corresponding to the last closed tag (ctrl-j)

Both are basically time-savers when editing XML documents.
Comment 1 Bert Vermeulen 2005-01-05 13:38:19 UTC
Created attachment 35472 [details]
XML helper plugin

This still needs "plugins/xmlhelp/Makefile" added to the AC_OUTPUT section in
configure.in in the dist root.
Comment 2 Paolo Borelli 2005-01-05 14:05:48 UTC
not a review by any standard (I just scrolled through the code...), but

- you use a lot of gedit_document_* functions, you should use gtk_text_view_*
functions instead (GeditDocument inherits from GtkTextView, those functions are
just leftovers from a lang time ago and some of them have been removed in
2.9.X). Note that this may require some rework of the code to operate using
GtkTextIter instead of offsets.

- Not a big deal, but it would be cool if you could use the same coding style of
the rest of gedit

- Not looked closely, but I see you include isspace etc... I seem to recall that
glib has equivalent functions which are utf8 safe.

- I spotted a if (not NULL) g_free, g_free alreday checks for NULL, so no need
fror the if


Btw, in the future please attach the file directly not gzipped, it makes take a
look at them easier.


On a more general note I was thinking that it would be cool to have a
"xml-bundle" plugin (or collection of plugins) which add more xml specific
tools, for instance a validator.
Comment 3 Bert Vermeulen 2005-01-05 15:46:55 UTC
- I will use the gedit internal API when you guys document what it is. Come to
think of it, the plugin API isn't documented either, and for that matter is
insufficient to write REALLY good plugins.

This XML helper is very crude because it depends on the user hitting a keyboard
shortcut or using the menu option. It really NEEDS to run live, and e.g. fill in
the last tag when you type "</". But there is no plugin hook that would let me
do stuff like that!

- I like my coding style fine, and it's not that different from other plugins.
I think only space before ()...

- isspace() is not used in the code

- checking for NULL is just good habit... just because g_free() is careful,
doesn't mean I don't need to be.

If you're thinking of proper XML support, I would consider that totally
unrelated to this plugin... this is really just like the name says, a simple XML
"helper" -- something to help you out when you need to type in a lot of XML. It
can do a lot more, and would if the plugin API permitted "live" actions, but I'd
still keep it separate from "real" XML stuff like well-formedness, validation,
doing clever things with DTDs etc.
Comment 4 Paolo Maggi 2005-01-05 16:20:49 UTC
You are right the gedit internal API is not documented and probably it is not
stable enough to write 3rd part plugins, but pbor only suggested you to use
GtkTextBuffer API insted of the GeditDocument one since we are going to clean
GeditDocument removing some of the functions that are already defined in the
parent class GtkTextBuffer.

You don't need plugin hooks to make your plugin run live. You only need to
connect to the GtkTextBuffer signal and see when the user type "</". The
automatic spell checker in the spell plugin may show you how to perform this
operation.

I like my coding style too, but when possible we prefer to adopt the official
GNOME coding style (see
http://developer.gnome.org/doc/guides/programming-guidelines/code-style.html)

Checking for NULL is a good habit, but why do you want to check twice?
From g_free documentation:
"Frees the memory pointed to by mem. If mem is NULL it simply returns."

Thanks,
Paolo M.
Comment 5 Paolo Borelli 2005-01-05 16:26:43 UTC
I guess I need to clarify some points:

- The XML bundle plugin I was talking about above was just a blue sky idea and
has nothing to do with the inclusion of your plugin. I was just stating that it
would be cool to collect a series of xml related functionalities :)

- about the "gedit internals api" I am exactly saying that you should NOT use
it: the plugin is passed a GeditDocument, which *is* a GtkTextBuffer, so you are
free to use all the gtk_text_buffer api as if you were writing your own editor.
That's pretty much as powerful as you can get, with regard to content manipulation.
If you need to use gtk_text_view apis you can easily obtain the view as well.
So to reiterate: try to not use gedit_document_*... in fact if you try to
compile against CVS head you'll find that some of the functions are gone.
The plugin api should only be needed to modify the gedit UI (and at the moment
it can only add menu items)

- about coding style: yes, I'm sure you like yours just fine, everybody does.
And I also know that it's really similar to the rest of the code (just the space
 before "(" and braces on new lines).
The fact is that if the plugin is to be included in gedit we would really prefer
to keep the coding style consistent across the codebase, since when we change
something in the plugin interface we have to fix all the plugins included.

- isspace is mentioned near an #include... that's why I pointed it out.

- g_free has been introduced *exactly* to automatically check for NULL, so if
(!NULL) before g_free is noise.
Comment 6 Paolo Borelli 2005-01-05 16:28:20 UTC
(just for the record, I submitted the comment above at the same time as Paolo M.
did, thus the redundancy :)
Comment 7 Bert Vermeulen 2005-01-08 08:06:08 UTC
Created attachment 35652 [details]
corrected xmlhelper

This replaces xmlhelp.c in the previous attachment.

This version uses no keyboard shortcuts or menu items; it automatically turns
itself on when an XML file is being edited, and does magic stuff as you type.

Also, all the gedit_document calls are gone, and it's all
GtkTextBuffer/iter/mark etc stuff.

Typing </ auto-closes the last open tag, and typing <> lets you cycle through
opening a new tag.
Comment 8 Paolo Maggi 2005-01-08 14:40:18 UTC
Comment on attachment 35652 [details]
corrected xmlhelper

Hi, 
  I have given a view to your code (but I have not tested it).
It seems good and, if it will prove to be robust enough when I will test it, I
think it could be included in the upcoming gedit 2.10.
I have only a few comments:
* there are still some style issues (minor problem we could fix just before
committing)
* reading the code I had the impression that it has some problem with
"undo/redo". May you please verify it?
* please, run it with valgrind and/or memprof to check for memleaks or other
problems (we can fix them also later)
* please, define a simbolic constant for the max size of the stack (256) (minor
issue)

The only showstop I see is the undo/redo problem we will have to test
carefully.

pbor: other comments?
Comment 9 Paolo Maggi 2005-01-08 14:42:09 UTC
ops... another commets.
I think it should work also when the language is HTML.
Comment 10 Paolo Maggi 2005-01-08 15:02:37 UTC
I have tested it and it seems to have some problems, i.e. I can make it crashes.

How to reproduce it:
1. Run gedit
2. Enable View->Highlight Mode->Markup->XML
3. Write <aaa></
4. It closes </aaa>
5. Write <bbb></
6. It closes </bbb>
7. Write <b
8. It crashes

I'm using CVS HEAD for gedit, gtksourceview and gtk+.

Here the stacktrace:

Backtrace was generated from '/opt/gnome/gnome210/INSTALL/bin/gedit'

Using host libthread_db library "/lib/tls/libthread_db.so.1".
[Thread debugging using libthread_db enabled]
[New Thread -151150464 (LWP 29491)]
0x0077b7a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2

Thread 1 (Thread -151150464 (LWP 29491))

  • #0 _dl_sysinfo_int80
    from /lib/ld-linux.so.2
  • #1 __waitpid_nocancel
    from /lib/tls/libpthread.so.0
  • #2 libgnomeui_segv_handle
    at gnome-ui-init.c line 741
  • #3 <signal handler called>
  • #4 IA__g_object_remove_weak_pointer
    at gobject.c line 1549
  • #5 queue_item_free
    at gdkgeometry-x11.c line 914
  • #6 gdk_window_queue
    at gdkgeometry-x11.c line 950
  • #7 _gdk_windowing_window_queue_antiexpose
    at gdkgeometry-x11.c line 1012
  • #8 gdk_window_process_updates_internal
    at gdkwindow.c line 2152
  • #9 IA__gdk_window_process_all_updates
    at gdkwindow.c line 2235
  • #10 gtk_container_idle_sizer
    at gtkcontainer.c line 1117
  • #11 g_idle_dispatch
    at gmain.c line 3821
  • #12 g_main_dispatch
    at gmain.c line 1947
  • #13 IA__g_main_context_dispatch
    at gmain.c line 2497
  • #14 g_main_context_iterate
    at gmain.c line 2578
  • #15 IA__g_main_loop_run
    at gmain.c line 2782
  • #16 IA__gtk_main
    at gtkmain.c line 963
  • #17 main
    at gedit2.c line 390
  • #0 _dl_sysinfo_int80
    from /lib/ld-linux.so.2

Yo
Comment 11 Paolo Maggi 2005-01-08 15:08:13 UTC
Another crash with similar steps but different stack trace:

Backtrace was generated from '/opt/gnome/gnome210/INSTALL/bin/gedit'

Using host libthread_db library "/lib/tls/libthread_db.so.1".
[Thread debugging using libthread_db enabled]
[New Thread -151150464 (LWP 29505)]
0x0077b7a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2

Thread 1 (Thread -151150464 (LWP 29505))

  • #0 _dl_sysinfo_int80
    from /lib/ld-linux.so.2
  • #1 __waitpid_nocancel
    from /lib/tls/libpthread.so.0
  • #2 libgnomeui_segv_handle
    at gnome-ui-init.c line 741
  • #3 <signal handler called>
  • #4 our_info.3
    from /opt/gnome/gnome210/INSTALL/lib/libgtk-x11-2.0.so.0
  • #5 ??
    from /opt/gnome/gnome210/INSTALL/lib/libpango-1.0.so.0
  • #6 values.175
    from /opt/gnome/gnome210/INSTALL/lib/libgtk-x11-2.0.so.0
  • #7 pango_attr_list_unref
    at pango-attributes.c line 993
  • #8 pango_layout_finalize
    at pango-layout.c line 180
  • #9 g_object_last_unref
    at gobject.c line 571
  • #10 IA__gtk_text_layout_free_line_display
    at gtktextlayout.c line 2150

<CUT>
Comment 12 Paolo Maggi 2005-01-08 16:25:21 UTC
Comment on attachment 35652 [details]
corrected xmlhelper

Hi, 
  I have other comments on your code.
Lemme concentrate on the close tag part for the moment.


>	/* automatically fill in end tag when </ is entered */
>	if (text[0] == '/')
>	{
>		prevpos = *position;
>		gtk_text_iter_backward_char (&prevpos);
>		buf = gtk_text_iter_get_visible_text (&prevpos, position);
>		if (g_utf8_get_char(buf) == '<')
>		{
>			gtk_text_iter_backward_char(&prevpos);
>			element = find_endable_tag(buffer, &prevpos);
>			if (element)
>			{
>				g_signal_handlers_block_by_func(buffer, buffer_insert_text_handler, NULL);
>				gtk_text_buffer_insert_at_cursor (buffer, "/", -1);
>				gtk_text_buffer_insert_at_cursor (buffer, element, -1);
>				gtk_text_buffer_insert_at_cursor (buffer, ">", -1);
>				g_signal_handlers_unblock_by_func(buffer, buffer_insert_text_handler, NULL);
>				g_free (element);
>				g_signal_stop_emission_by_name (buffer, "insert-text");
>			}
>		}

I think this part of code is a bit dangerous... I'd prefer a solution that does
not play too much with blocking/unblockin signals.
Instead of catching the "insert-text" signal of the document, I think you
should catch the "key_press_event" of the view and
when the "/" key is pressed, check if the character just before the "insert"
mark is '<'. In that case, instead of adding
"/" you will add as a single operation the closing tag. In this way you will
not have problems with the undo manager (that is working on the insert signal
of the document). In the case you have added the tag, then the handler of
"key_press_event" will have to return TRUE, otherwise FALSE.
For an example of how to use this technique give a look at "key_press_cb"
method in gtksourceview.c
(http://cvs.gnome.org/viewcvs/gtksourceview/gtksourceview/gtksourceview.c?rev=1
.30&view=markup).

Other comments to come...

Regards,
Paolo
Comment 13 Bert Vermeulen 2005-01-09 02:25:20 UTC
Created attachment 35696 [details]
new xmlhelp.c

This version:

- works with undo/redo
- seems ok with valgrind (please check, I don't know valgrind very well)
- has a symbol for the stack size
- has the segfault fixed
- uses "key_press_event" instead of "insert-text"
- doesn't try to close processing instructions
Comment 14 Bert Vermeulen 2005-01-10 00:37:42 UTC
Created attachment 35768 [details]
XML helper plugin

same version, but a complete list of the new files, plus a patch for
configure.in
Comment 15 Paolo Maggi 2005-01-10 00:42:13 UTC
<biot> I disagree about turning it on for HTML as well
<paolo> why?
<biot> it's really quite XML-specific, at least the auto-close thing
<biot> since with HTML, you don't *have* to close tags
<biot> hence, you can't assume that the tag you're closing is simply the last
opened one
<biot> some people might find it handy, but most would not... since most people
write sloppy HTML
<biot> so at most, something like a way to turn it on for HTML files, but not
automatic
<paolo> hmmm... I think there should be a way to temporaly escape the tag closing
<paolo> what does it happens in the following case:
<paolo> hmmm.. nothing wrong example
<biot> perhaps a menu item so you can enable the plugin, if your file is HTML
<biot> although that's not very intuitive
<paolo> no, I was thinking something like pressing the ESC key if you don't want
the plugin close the tag
<paolo> for example while you are editing a CDATA
<paolo> actually it should be automatically disabled while you are editing a CDATA
<biot> I don't get what you mean
<paolo> suppose you want to edit:
<paolo> <a>bbb</a><![CDATA[test: </b>]]
<paolo> you cannot if the XMLhelp plugin is active
<paolo> am I smoking crack?
<biot> you mean typing in the </b> wouldn't work?
<biot> (it does...)
<paolo> does it detect there is not open tag?
<biot> yes
<biot> if there's nothing there to fill in, it just doesn't do anything, you
keep typing
<paolo> and this: <a>bbb</a><![CDATA[test: <a></b>]]
<biot> wait
<biot> I'm wrong about the first one, it closed that <!CDATA
<biot> I have no idea what <!CDATA means, should probably find out 
<biot> :)
<paolo> CDATA
<paolo> 
<paolo> Everything inside a CDATA section is ignored by the parser.
<paolo> 
<paolo> If your text contains a lot of "<" or "&" characters - as program code
often does - the XML element can be defined as a CDATA section.
<paolo> 
<paolo> A CDATA section starts with "<![CDATA[" and ends with "]]>
<paolo> http://www.w3schools.com/xml/xml_cdata.asp
<biot> yeah, just looking at the spec now
<biot> figuring out if you're in a CDATA section is going to involve quite a bit
of parsing
<biot> not like the quick tricks I do now
<paolo> well, if syntax hl is enabled there will be a trick, i.e. seeing if you
are inside a syntax region tag called "CDATA"
<biot> ah, gtksourceview provides functionality for this?
<paolo> yep, it can help 
<paolo> but it is a bit hacky
[...]
<paolo> biot: I'm going to attach the IRC log to the bug report. Is it ok for you?
<biot> about the HTML thing?
<paolo> and the CDATA one
<biot> ok
Comment 16 Paolo Maggi 2005-01-11 10:33:12 UTC
*** Bug 154080 has been marked as a duplicate of this bug. ***
Comment 17 Rowan Lewis 2005-03-12 08:04:31 UTC
Has any more work gone into the plugin? Or will something like it be written later?
Comment 18 Bert Vermeulen 2005-03-15 04:48:39 UTC
Well, it works as advertised. It's up to Paolo to decide whether he wants it in
gedit or not.
Comment 19 Alan Horkan 2005-03-15 04:57:05 UTC
I figure that (while I'm removing myself from the CC list) I may as well move 
this to the New state and add the PATCH keyword.   
Comment 20 Kjartan Maraas 2005-09-19 13:04:09 UTC
The backtrace in comment #10 is the same as a gnome-terminal crash reported in
bug  151680
Comment 21 Paolo Borelli 2005-09-19 13:33:13 UTC
Hello Biot ;)

this is far from WONTIFIX even if we haven't had time to get back to it since we
are still working on the new plugin system etc. reopening.

If you don't have time/will to work on the plugin that's fair enough, but I
think the bug should stay open as it contains useful info for anyone who wants
to write this feature.
Comment 22 Bert Vermeulen 2005-09-19 13:37:26 UTC
Then please remove my email from the list... I wasn't sure how to do that, since
I'd originally opened this bug.

It's obvious to me that no action will be taken on this, hence I've lost interest.
Comment 23 Paolo Borelli 2005-09-19 13:56:34 UTC
Bert, I'm sorry but I don't think it's possible to remove your mail address
since as you say you are the original reporter and bugzilla doesn't allow it.
Given the lack of activity on this bug in recent times, I hope you can put up
with some random mails... at worse you can use procmail.

I'm also sorry for the lack of action on the plugin and I understand your
frustration, but we have very limited manpower and we don't work on gedit for a
job. Our resources have been put elsewere
(http://live.gnome.org/Gedit_2fNewMdi). When that major work is done, we hope to
get more traction on plugins especially considering that writing them in python
will be supported.

Once again, sorry for having wasted some of your time, I at least hope that in
the meantime your plugins has been useful for you. (By the way nothing stops you
from releasing it as a separate package).


ciao,
    paolo

Comment 24 Paolo Maggi 2006-12-30 17:59:27 UTC
We are moving all the request for plugins that we don't plan to add to gedit
itself on http://live.gnome.org/Gedit/Plugins.

Closing as WONTFIX.

Comment 25 Matěj Cepl 2007-11-26 10:21:22 UTC
Now (hopefully) fully obsoleted by Python Gedit plugin available on http://live.gnome.org/Gedit/XMLHelperPlugin.