[Geany-devel] rewritten load_startup_files()

[Eugene Arshinov]

On Wed, 13 Apr 2011 21:51:17 +0300%
Dimitar Zhekov <dimitar.zhekov at gmail.com> wrote:

> Hi (sigh),
> 
> My first implementation was a bit naive. With "Project based session
> files" turned off, specifying a CL project should have absolutely no
> effect on the session loading. In particular, not only --no-session,
> but any of "Load files from the last session" = off, --new-instance,
> or CL file names after the project name should block the session.
> 
> Not much of a difference though...
> 

Hi.

I'd like to vote for including this patch.  First of all, because it
makes Geany behave ALMOST EXACTLY like it does in SM branch :D


I must note that this version print GTK assertion in a couple
of cases.  This happens when user tries to (1) simultaneously open a
project and load the default session and at the same time (2) has "Use
project-based project files" preference unchecked.  In these conditions
the code calls (in the following order):

  main_load_project_from_command_line();
  load_session_project_file();
  configuration_open_files();
  open_cl_files();

Function load_config() called by load_session_project_file() checks if
any project is already loaded.  The check fails, and load_config() does
nothing except printing the assertion.  So, the solution will be not to
call load_session_project_file() here (as it does not do anything
anyway, and default session seems to open even without this call).


The second thought is that the code in this patch and in SM branch is
QUITE different (although it behaves the same).  Of course, I would
prefer if Dimitar

1. took SM version
2. changed the code to get project filename from `argv' instead of new
   command-line option (cl_options.project) added in SM
2. cut off the unnecessary checks (**)
3. add a check for new_instance (in SM it implies load_session=false,
   so in my code I don't need to explicitly check new_instance)

In this case the resulting code would be more similar to existing
SM's…  But (as you can see) there are maybe too many modifications that
can introduce bugs (which are hard to find as all testing, I guess, is
manual), so maybe it was a right decision to rewrite load_startup_files
again from "scratch" (I mean, the trunk version :D).

What I was trying to say is that I see three choices here
1. This patch is ignored… (so the behaviour remains a little buggy and
   inconvenient, as discussed by Dimitar and Lex, and different from SM's);
2. This patch is included in trunk and maybe (I am not sure it's worth
   doing it) I try to modify SM's version so it looks more similar and
   familiar for other developers who have already seen this patch's
   version (it will make sense if anyone anytime wants to dive into SM's
   version).
3. Someone takes SM version and cuts it down like I described before.
   I see little sense in doing it, as Dimitar has already prepared a
   nice patch which works fine (except the asserion) and is generally
   perfect (except it's code is different from SM's :D, oh I'm
   repeating myself).

So, repeating what I said in the first paragraph, I vote for including
this patch in trunk (choice #2).


** In SM there is additional handling of:
- if Geany is being restored by the session manager
- new load_session_if_any_files_loaded preference


--
Best regards,
Eugene.