Re: [Geany-devel] Proposed patch to fix issues with command line file loading

2011-02-26 Thread Frank Lanitz
On Mon, 31 Jan 2011 09:06:26 +1100
Lex Trotman ele...@gmail.com wrote:

 On 31 January 2011 02:54, Frank Lanitz fr...@frank.uvena.de wrote:
  On Fri, 28 Jan 2011 11:31:21 +0300
  Eugene Arshinov earshi...@gmail.com wrote:
 
  On Thu, 27 Jan 2011 10:20:26 +0100%
  weltall welta...@gmail.com wrote:
 
   Hi,
   I've noticed various issues with file loading from command line:
   1) if the option load last session is not enabled project files
   can't be opened from command line: this happens because the code
   to handle them is wrapped in a prefs.load_lastsession if so the
   only way to load them from command line is having that option
   true.
   2) trying to open a file (eg from double clicking in nautilus)
   will screw your session as command line loading of single files
   ignores your session opened files, except if geany was already
   opened.
  
   So to fix those issues I've reorganized the code in the
   load_startup_file function in order to:
   1) load the project files ignoring the session if a project file
   was specified at command line
   2) allow to load more files in addition to the project file
   (removing so the limitation imposed there artificially - possible
   improvement to this would be adding a flag to open_cl_files as
   argument in order to avoid that pointer toying)
   3) the open_cl_files is called after loading the session so the
   specifically selected files are opened last and so are
   automatically selected (what you would expect when opening a
   file from nautilus)
   4) if the option to load last session is not enabled and there
   isn't a project file being loaded from command line and a project
   is not being loaded we just try to load files from command line
   if any
  
   Stefano Angeleri
 
  Just to mention, I had similar objections about a year ago when I
  wrote my version of session management support (the list probably
  knows what I'm talking about).  This was discussed a little, and
  some patches were included in 'sm' branch.  For example,
  load_startup_files () function in that branch differs
  significantly from trunk, and open_cl_files() does not have a
  check for `argc = 1`.  Geany's SVN browse seems to be unavailable
  now, but I think it should be possible there to view 'sm' branch.
 
  The branch is still available. To checkout its
 
  svn co https://geany.svn.sourceforge.net/svnroot/geany/branches/sm
 
  The sf viewer seems to be really down at the moment.
 
 
  It is harder to find the discussion than the patches themselves,
  but here is a couple of links:
 
  http://lists.uvena.de/pipermail/geany-devel/2009-November/001577.html
  (X session management support)
  http://lists.uvena.de/geany-devel/2010-January/001655.html
  (Questions about Geany project support)
 
  I don't object against your patches, but I'm just pointing to
  existing discussion which may be useful.  I must note that there
  are no plans about including those patches from 'sm' branch in
  trunk (because nobody would want/have time to do it, and it won't
  be quite easy as 'sm' branch has become rather diverse from
  trunk), so your way is free :)
 
  Well, not sure whether I'm right the branch wasn't ready for
  including to trunk and nobody did care about later on. Please
  correct me if I'm wrong.
 
 
 I guess now (post 0.20) is the time to do major additions, if a
 current trunk is merged with sm branch and it still works then maybe
 its time.

I guess this is something the original author of the branch needs to
do. 

Cheers, 
Frank
-- 
http://frank.uvena.de/en/


pgpEuotRH6nr3.pgp
Description: PGP signature
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


Re: [Geany-devel] A few fixes for codenav, geanygdb, geanyinsertnum and geanylatex

2011-02-26 Thread Colomban Wendling
Le 26/02/2011 13:41, Frank Lanitz a écrit :
 On Wed, 23 Feb 2011 19:32:07 +0100
 Colomban Wendling lists@herbesfolles.org wrote:
 
 [./geanylatex/src/templates.c:47]: (error) Memory leak: template

 I also attach here the fixes I propose for them, if you're interested.
 
 I'm afraid I don't see why this should be a memory leak. Can you please
 be so kind and go into more details to prevent similar cases in future?
It's a possible memory leak because you exit the function conditionally
after the allocation, but don't free it in this branch:

TemplateEntry *template = g_new0(TemplateEntry, 1);
gchar *tmp = NULL;

/* Return if its not a searched file */
if (g_str_has_suffix(file,.gtl) == FALSE)
/* Leak here! */
return;

/* ... */

g_free(template);

So the easiest fix is to allocate the memory after the conditional exit
since it isn't used before.

Regards,
Colomban
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


Re: [Geany-devel] A few fixes for codenav, geanygdb, geanyinsertnum and geanylatex

2011-02-26 Thread Colomban Wendling
Le 26/02/2011 14:50, Frank Lanitz a écrit :
 On Sat, 26 Feb 2011 14:21:11 +0100
 Colomban Wendling lists@herbesfolles.org wrote:
 
 Le 26/02/2011 13:41, Frank Lanitz a écrit :
 On Wed, 23 Feb 2011 19:32:07 +0100
 Colomban Wendling lists@herbesfolles.org wrote:

 [./geanylatex/src/templates.c:47]: (error) Memory leak: template

 I also attach here the fixes I propose for them, if you're
 interested.

 I'm afraid I don't see why this should be a memory leak. Can you
 please be so kind and go into more details to prevent similar cases
 in future?
 It's a possible memory leak because you exit the function
 conditionally after the allocation, but don't free it in this branch:

  TemplateEntry *template = g_new0(TemplateEntry, 1);
  gchar *tmp = NULL;

  /* Return if its not a searched file */
  if (g_str_has_suffix(file,.gtl) == FALSE)
  /* Leak here! */
  return;

  /* ... */

  g_free(template);

 So the easiest fix is to allocate the memory after the conditional
 exit since it isn't used before.
 
 True. 
 
 Ah... and now I see. You were not doing a patch against trunk of
 geanylatex. Its fixed already inside http://yaturl.net/2c80 
Oops, I didn't though main development don't go in
geany-plugins/geanylatex, sorry :/

However, I suggest you to do the allocation after the conditional exit
rather than before and freeing it if taking the branch, because
alloc/free is far from being a no-op ;)

Cheers,
Colomban
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


Re: [Geany-devel] A few fixes for codenav, geanygdb, geanyinsertnum and geanylatex

2011-02-26 Thread Frank Lanitz
On Sat, 26 Feb 2011 16:45:13 +0100
Colomban Wendling lists@herbesfolles.org wrote:

 Le 26/02/2011 14:50, Frank Lanitz a écrit :
  On Sat, 26 Feb 2011 14:21:11 +0100
  Colomban Wendling lists@herbesfolles.org wrote:
  
  Le 26/02/2011 13:41, Frank Lanitz a écrit :
  On Wed, 23 Feb 2011 19:32:07 +0100
  Colomban Wendling lists@herbesfolles.org wrote:
 
  [./geanylatex/src/templates.c:47]: (error) Memory leak: template
 
  I also attach here the fixes I propose for them, if you're
  interested.
 
  I'm afraid I don't see why this should be a memory leak. Can you
  please be so kind and go into more details to prevent similar
  cases in future?
  It's a possible memory leak because you exit the function
  conditionally after the allocation, but don't free it in this
  branch:
 
 TemplateEntry *template = g_new0(TemplateEntry, 1);
 gchar *tmp = NULL;
 
 /* Return if its not a searched file */
 if (g_str_has_suffix(file,.gtl) == FALSE)
 /* Leak here! */
 return;
 
 /* ... */
 
 g_free(template);
 
  So the easiest fix is to allocate the memory after the conditional
  exit since it isn't used before.
  
  True. 
  
  Ah... and now I see. You were not doing a patch against trunk of
  geanylatex. Its fixed already inside http://yaturl.net/2c80 
 Oops, I didn't though main development don't go in
 geany-plugins/geanylatex, sorry :/
 
 However, I suggest you to do the allocation after the conditional exit
 rather than before and freeing it if taking the branch, because
 alloc/free is far from being a no-op ;)

Yep, you are right. Will do this ;) 

Cheers, 
Frank 
-- 
http://frank.uvena.de/en/


pgpTvlJRMaBhJ.pgp
Description: PGP signature
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel