Bug 13075 - Several xedit fixes
Summary: Several xedit fixes
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: App/xedit (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-03 23:09 UTC by Paulo César Pereira de Andrade
Modified: 2008-07-10 10:55 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
xedit patch (129.16 KB, patch)
2007-11-03 23:11 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
Ignore xedit.patch use only xedit-1.patch (137.30 KB, patch)
2007-11-03 23:56 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
fix ispell interface, defaults to noxprint to reenable syntax hightlight, regex, etc, several bugs and add tags interface (149.06 KB, patch)
2007-11-06 04:10 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
Latest patch version. Single file. Obsoletes previous posted patches in this bug report. (46.54 KB, application/octet-stream)
2007-11-16 06:00 UTC, Paulo César Pereira de Andrade
no flags Details
Latest version. Obsoletes previous posted patches. (52.79 KB, application/octet-stream)
2008-02-11 20:33 UTC, Paulo César Pereira de Andrade
no flags Details
0016-Fix-an-incorrect-buffer-size-calculation-and-allocat.patch (1.37 KB, patch)
2008-02-12 21:33 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
0014-Add-perl-and-auto-tools-modes.patch (23.89 KB, patch)
2008-02-13 18:31 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
xedit-patches.tar.bz2 (43.62 KB, application/octet-stream)
2008-03-11 18:39 UTC, Paulo César Pereira de Andrade
no flags Details
xedit-patches.tar.bz2 (53.28 KB, application/octet-stream)
2008-03-12 18:09 UTC, Paulo César Pereira de Andrade
no flags Details
0018-Compile-warning-fixes.patch (1.37 KB, patch)
2008-03-16 16:54 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
xedit-patches.tar.bz2 (55.35 KB, application/x-bzip)
2008-04-12 13:53 UTC, Paulo César Pereira de Andrade
no flags Details
0020-Warn-if-a-newer-version-of-a-file-exists-before-over.patch (7.11 KB, patch)
2008-05-10 17:42 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
0021-Fix-an-off-by-one-error-check-that-can-lead-to-an-in.patch (942 bytes, patch)
2008-05-10 17:43 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review

Description Paulo César Pereira de Andrade 2007-11-03 23:09:46 UTC
I use xedit for programming, with some local patches, to reenable features that were disabled with the xprint support. I have been in the Windows world for some years, but now that I am working at Mandriva, I started using xedit again.
  I found that having a tags file opened in either vi or another xedit window was painfull, so I wrote a tags interface to xedit, as well as fixed all the problems I could notice in the Xorg version.
  This patch should work on any system that already supports xedit.
  The Changelog file update lists all changes. The README and INSTALL files should also describe very well the changes in this patch as well as document a lot of other xedit features.

  Note that this patch also disables xprint by default. I wish this were not the case, but unfortunatelly, I don't use xprint, and don't feel like fixing all the code that isn't multibyte ready. I use xedit only for programming, and don't need multibyte for it... Maybe extending it to handle utf-8 would be somewhat easy (or at least add the required interfaces xprint requires), but could require some Xaw changes also. Xaw9 anyone? >:}
Comment 1 Paulo César Pereira de Andrade 2007-11-03 23:11:02 UTC
Created attachment 12341 [details] [review]
xedit patch
Comment 2 Paulo César Pereira de Andrade 2007-11-03 23:56:06 UTC
Created attachment 12342 [details] [review]
Ignore xedit.patch use only xedit-1.patch
Comment 3 Paulo César Pereira de Andrade 2007-11-06 04:07:53 UTC
  Sunday I made another patch that I should apply on Mandriva packages, removing
all other Mandriva patches soon.
  I used the default autotools CFLAGS in xedit-1.patch, but when compiling with
-Wall there is a lot of warnings, so I changed one structure field from "unsigned char *" to "char *" to remove 95% of the warnings, and also fixed a
few problems where the conversion of "char *" to "struct _hash_key *" was not
properly updated and a pointer to structure was being passed to a printf "%s"
argument in case of errors.
  Note that this patch isn't incompatible with the xprint support, just that it
disables it by default, and it isn't a runtime option (but could also be
changed to it, if there is interest). I may write a patch to "make xprint
happy" if there is interest, but that could require a very large amount of
work. Could be interesting to print sources with syntax hightlight and the
like.
  Unless I modify the tags code to handle multiple tags (I am considering this
to make navigating in different projects easier), i.e. building a tree of tag
structures and associating a tags file with the file being edited, and
choosing/searching the topmost tags file when opening a new file, this should
be my last patch for xedit for a long time.
Comment 4 Paulo César Pereira de Andrade 2007-11-06 04:10:23 UTC
Created attachment 12370 [details] [review]
fix ispell interface, defaults to noxprint to reenable syntax hightlight, regex, etc, several bugs and add tags interface
Comment 5 Paulo César Pereira de Andrade 2007-11-16 06:00:19 UTC
Created attachment 12595 [details]
Latest patch version. Single file. Obsoletes previous posted patches in this bug report.

  This is a single patch, that obsoletes previous patches. Sorry, I did not obsolete others when starting posting patches... This patch has several updates
to the Mandriva Cooker xedit version, that should be also updated soon.
Minor changelog is:
o Update README, COPYING, ChangeLog, etc
o Update ispell interface, should work on multi byte and single byte locales,
  for single byte this is basically an "english" spell checker, as xedit
  doesn't properly handle utf8 in non multibyte mode. Not sure if it is worth
  working on it.
o Added mouse wheel actions for scrolling the text window
o Default to no-xprint support. This is another thing that I am not sure if is
  worth fixing.
o Check all calls to XeditPrintf (the original version wasn't a printf_format)
  function, and Xorg conversion to such raised some problems, as the code would
  "sprintf" a string argument, that could have printf special characters, like
  filenames with % and * character, and such.
o Minor fix to filenamewindow that would not update the horizontal scroll when
  the text value were changed with XtSetValues. This probably should be fixed
  in Xaw.
o Fixed an issue when replacing ~username where it would hold a pointer to a
  substring, but then realloc the base pointer, what could cause random
  crashes.
o Fixed assumption that '.' and '..' are the first two directory entries.
o Add a generic hash table to replace all other hash table implementations
  inside xedit.
o Fixed problem in the line-edit-mode that would not properly match patterns
  in the last line of a file due to buggy check for files ending with newline.
o Some other minor fixes to line-edit-mode like wrong substitution for
  escaped characters.
o Added a tags interface. This should work properly with 'exuberant-ctags'
  (default on Linux distros), when run as "ctags -R". When loading a new
  file, the interface descends from the file directory up to the root
  directory searching for a tags file. When searching the definition of a
  symbol, it first tries the "best" match tags that is already associated
  with the current buffer, and if failing, tries all other loaded tags, in
  no specific order; this allows for example, finding library definitions
  from sources a different project.
  I am considering a different approach for it, as for example, Linux kernel
  2.6.x has an almost 80Mb tags file, and xedit uses almost 100Mb to store
  the tags information, and takes significant time to load. A better way
  would be to have something like a binary tags file that would allow a very
  fast lookup algorithm to be used, so that memory usage would be minimal, and
  symbol defintion searches would be quite faster. Could also be extended to
  not only find the symbol definition, but also search other uses of the symbol.
  Maybe gnu global could be the proper application to handle it.
o The XeditPrintf (that prints text to the messagewidget) now also checks for
  repeated messages, and like gnu emacs, prints the ammount of times is has
  repeated.
o Manpage documentation of new resources tagsName (String) and loadTags
  (Boolean)
Comment 6 Paulo César Pereira de Andrade 2008-02-11 20:33:25 UTC
Created attachment 14280 [details]
Latest version. Obsoletes previous posted patches.

git-format-patch from a branch where patches were rebuilt
to address an issue at a time, instead of a single big blob.

Please git-am to xorg/app/xedit

Take special attention to patch 14 as it adds two GPL
licensed files, but the entire patch can be skiped
without problems.
Comment 7 Alan Coopersmith 2008-02-11 20:36:45 UTC
Would you like to be xedit maintainer?    I don't think anyone else has
as much interest in it as you seem to.
Comment 8 Paulo César Pereira de Andrade 2008-02-11 21:05:50 UTC
  Surely I want to be the xedit maintainer :-)

  My major interest is just to have it functional,
as I wrote most of it's code.
Comment 9 Julien Cristau 2008-02-11 21:29:55 UTC
(In reply to comment #6)
> Take special attention to patch 14 as it adds two GPL
> licensed files, but the entire patch can be skiped
> without problems.
> 
i'm not quite sure i see the point of changing the license for two trivial files. can you clarify why they're not under the same license as the rest of xedit?
Comment 10 Paulo César Pereira de Andrade 2008-02-12 11:07:55 UTC
(In reply to comment #9)
> (In reply to comment #6)
> > Take special attention to patch 14 as it adds two GPL
> > licensed files, but the entire patch can be skiped
> > without problems.
> > 
> i'm not quite sure i see the point of changing the license for two trivial
> files. can you clarify why they're not under the same license as the rest of
> xedit?

  All code I contributed to XFree86 I either did not change any copyrights
on existing files or used XFree86 license on new files.

  I added two new files with a BSD license and copyright to myself.

  The new files are not mandatory, so I used the GPL license.

  Actually, I wrote the perl mode to edit the "xorg-scripts" I posted some
days ago in the xorg mailing list, and the "auto" mode for syntax highlight
when editing configure.ac.

  The tags interface is also useful to work with the several different
xorg git repositories, and very useful when used with the
xorg-scripts/xorg-symbols.pl script.

  I could use some other editor that already has tags or a "perl" mode, but
I use only xedit, unless I am distracted and type vi in a xterm :-)
Comment 11 Peter Hutterer 2008-02-12 19:54:25 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #6)
> 
>   All code I contributed to XFree86 I either did not change any copyrights
> on existing files or used XFree86 license on new files.
> 
>   I added two new files with a BSD license and copyright to myself.
> 
>   The new files are not mandatory, so I used the GPL license.

I don't understand why you would choose a different license for this change. I do know the benefits of GPL versus BSD, but in this case it seems trying to force xedit to become GPL.
Is it really worth essentially forking xedit over these changes?
Comment 12 Paulo César Pereira de Andrade 2008-02-12 21:19:51 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #6)
> > 
> >   All code I contributed to XFree86 I either did not change any copyrights
> > on existing files or used XFree86 license on new files.
> > 
> >   I added two new files with a BSD license and copyright to myself.
> > 
> >   The new files are not mandatory, so I used the GPL license.
> 
> I don't understand why you would choose a different license for this change. I
> do know the benefits of GPL versus BSD, but in this case it seems trying to
> force xedit to become GPL.
> Is it really worth essentially forking xedit over these changes?

  Sorry, I am really not trying to gratuitously complicate things.

  The files I changed license were written long after (well more than one
month :-) I posted the first patch here, but one could do something like
 'git-am `ls 00* | grep -v 0014`' (or grep -v GPL)

  I am not a lawyer, neither never really stopped for too much time to
try to understand about licensing/copyright issues. If this is really an
issue I can change the license of those files.

  It is just that for quite some time almost all code I write in my free
time I license it under GPL, but add the copyright to myself (at least
until I have something worth, and contact FSF to assign copyright).
I just want any possible derivate work to have source code available.

  But I don't plan to do any major work on xedit, so I am fine on ensuring
any new code uses a BSD like license.
Comment 13 Paulo César Pereira de Andrade 2008-02-12 21:33:32 UTC
Created attachment 14294 [details] [review]
0016-Fix-an-incorrect-buffer-size-calculation-and-allocat.patch

  This patch complements patches in xedit-patches.tar.bz2

  The bug this patches fixes isn't really easy to trigger (needs
consecutive writes larger than pagesize to a string buffer), but
is a plain oversight and wrong code.
Comment 14 Peter Hutterer 2008-02-13 00:13:54 UTC
(In reply to comment #12)
>   I am not a lawyer, neither never really stopped for too much time to
> try to understand about licensing/copyright issues. If this is really an
> issue I can change the license of those files.

Essentially it comes down that if you put these changes in, xedit is automatically GPL. That's how I understand it anyway.

>   It is just that for quite some time almost all code I write in my free
> time I license it under GPL, but add the copyright to myself (at least
> until I have something worth, and contact FSF to assign copyright).
> I just want any possible derivate work to have source code available

If you are the only author you can just relicense your patches before applying them. 
 
>   But I don't plan to do any major work on xedit, so I am fine on ensuring
> any new code uses a BSD like license.
 
I understand the reasons why you would start new code in GPL. When working on an existing project, it is usually beneficial and a sign of respect to the original authors to retain their choice of license.
Of course, this isn't a black and white decision, but unless you plan to really revamp the app I wouldn't recommend a change in the license.

Of course, all this for this project only.
Comment 15 Paulo César Pereira de Andrade 2008-02-13 18:29:24 UTC
(In reply to comment #14)

> If you are the only author you can just relicense your patches before applying
> them. 

  I wrote over 95% of the current code. And I was careful to avoid changing
the original code (that dates back to 1987), and was basically a front end
to the Xaw text widget.

> >   But I don't plan to do any major work on xedit, so I am fine on ensuring
> > any new code uses a BSD like license.
> 
> I understand the reasons why you would start new code in GPL. When working on
> an existing project, it is usually beneficial and a sign of respect to the
> original authors to retain their choice of license.

  I posted a reduced version of this patch in 2004, since then, when using
some system with Xorg, I would just recompile xedit.

> Of course, this isn't a black and white decision, but unless you plan to really
> revamp the app I wouldn't recommend a change in the license.

  It would be better to write a new editor from scratch :-)

> Of course, all this for this project only.
Comment 16 Paulo César Pereira de Andrade 2008-02-13 18:31:37 UTC
Created attachment 14302 [details] [review]
0014-Add-perl-and-auto-tools-modes.patch

Replaces 0014-GPL-licensed-perl-and-auto-tools-modes.patch
in the xedit-patches.tar.bz2 file, adding new files with a
standard BSD like license.
Comment 17 Paulo César Pereira de Andrade 2008-03-11 18:39:24 UTC
Created attachment 15046 [details]
xedit-patches.tar.bz2

  Latest version of patches.

  This patch obsoletes all the previous ones.

  It also fixes a problem where, when spliting in commits
that address one issue per patch, one of the patches had
been overwritten.

  Also add a new patch to allow multiple make jobs, not
really important for a small package, mainly to avoid
xedit requiring some special rule where something like
"make -j 16" is the default.
Comment 18 Paulo César Pereira de Andrade 2008-03-12 18:09:22 UTC
Created attachment 15077 [details]
xedit-patches.tar.bz2

  Err, when making a single tarball to store all patches,
I also tried to be smart and edited patches to remove
spaces errors, i.e. do something like 's@\s+$@@' and then:
$ git-apply <patch.file>
$ git-commit -a
<cut&past of commit message in previous patch>

but I forgot that git-apply doesn't automatically add new
files, and only noticed the problem when making a checkout
of my "patches" branch elsewhere and trying to rebuild.

Fixed adding new files now, and git-am should not report
space errors (i.e. new lines with spaces before eol).
Comment 19 Paulo César Pereira de Andrade 2008-03-16 16:54:45 UTC
Created attachment 15217 [details] [review]
0018-Compile-warning-fixes.patch

  Add parenthesis around a test where after macro expansion it looked like
boolexpr==boolres==boolres.
  "Ansifiy" a function without arguments.

  This patch should work if applied in any order, but for better results,
use it after the patches attachment #15077 [details].
Comment 20 Paulo César Pereira de Andrade 2008-04-12 13:53:16 UTC
Created attachment 15851 [details]
xedit-patches.tar.bz2

  Latest version of patches. Should be ready to git-am,
git-apply or simply patch.

  The change from the previous tarball is that is adds
patch 18 to it, and the new patch19 that is a python
mode, with syntax highlight and automatic indentation.
Comment 21 Paulo César Pereira de Andrade 2008-05-10 17:42:59 UTC
Created attachment 16468 [details] [review]
0020-Warn-if-a-newer-version-of-a-file-exists-before-over.patch

  Print a warning and request a second click to overwrite
a file if a newer version exists.

  This patch complents the patches in the tarball.
Comment 22 Paulo César Pereira de Andrade 2008-05-10 17:43:52 UTC
Created attachment 16469 [details] [review]
0021-Fix-an-off-by-one-error-check-that-can-lead-to-an-in.patch

Fix an off by one error check that can lead to an infinite loop.

This can happen when using the line edit mode to search&replace regexes.

This patch complents the patches in the tarball.
Comment 23 Paulo César Pereira de Andrade 2008-07-10 10:55:48 UTC
Patches have been commited.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.