Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for "Open in New Window". #637

Closed
wants to merge 1 commit into from
Closed

Fix for "Open in New Window". #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/libmain.c
View file
Open in desktop
Expand Up @@ -113,7 +113,7 @@ static gboolean no_plugins = FALSE;
static gboolean dummy = FALSE;

/* in alphabetical order of short options */
static GOptionEntry entries[] =
GOptionEntry optentries[] =
{
{ "column", 0, 0, G_OPTION_ARG_INT, &cl_options.goto_column, N_("Set initial column number for the first opened file (useful in conjunction with --line)"), NULL },
{ "config", 'c', 0, G_OPTION_ARG_FILENAME, &alternate_config, N_("Use an alternate configuration directory"), NULL },
Expand Down Expand Up @@ -141,10 +141,41 @@ static GOptionEntry entries[] =
{ "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose_mode, N_("Be verbose"), NULL },
{ "version", 'V', 0, G_OPTION_ARG_NONE, &show_version, N_("Show version and exit"), NULL },
{ "dummy", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &dummy, NULL, NULL }, /* for +NNN line number arguments */
/* add new options here *and* in `optentries_aux' below */
{ NULL, 0, 0, 0, NULL, NULL, NULL }
};


GeanyOptionEntryAux optentries_aux[] = {
{FALSE}, /* "column" */
{TRUE}, /* "config" */
{FALSE}, /* "ft-names */
{FALSE}, /* "generate-tags" */
{FALSE}, /* "no-preprocessing" */
#ifdef HAVE_SOCKET
{FALSE}, /* "new-instance" */
{TRUE}, /* "socket-file" */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't not having a "--new-instance" but pointing to the same socket file make it open in the same instance, not start a new one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FALSE = "when reproducing the command line options passed to this instance to launch a New Window, ignore this option".

Since "-i" is automatically appended in utils_start_new_geany_instance(), it doesn't matter whether it's passed to this (launching) instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the -i makes the new geany ignore the socket, so why pass it :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I took it from the SM, and only corrected the "new-instance". :)

{FALSE}, /* "list-documents" */
#endif
{FALSE}, /* "line" */
{TRUE}, /* "no-msgwin" */
{TRUE}, /* "no-ctags" */
#ifdef HAVE_PLUGINS
{TRUE}, /* "no-plugins" */
#endif
{FALSE}, /* "print-prefix" */
{FALSE}, /* "read-only" */
{FALSE}, /* "no-session" */
#ifdef HAVE_VTE
{TRUE}, /* "no-terminal" */
{TRUE}, /* "vte-lib" */
#endif
{TRUE}, /* "verbose" */
{FALSE}, /* "version" */
{FALSE} /* "dummy" */
};


static void setup_window_position(void)
{
/* interprets the saved window geometry */
Expand Down Expand Up @@ -514,7 +545,7 @@ static void parse_command_line_options(gint *argc, gchar ***argv)

context = g_option_context_new(_("[FILES...]"));

g_option_context_add_main_entries(context, entries, GETTEXT_PACKAGE);
g_option_context_add_main_entries(context, optentries, GETTEXT_PACKAGE);
g_option_group_set_translation_domain(g_option_context_get_main_group(context), GETTEXT_PACKAGE);
g_option_context_add_group(context, gtk_get_option_group(FALSE));
g_option_context_parse(context, argc, argv, &error);
Expand Down
11 changes: 11 additions & 0 deletions src/main.h
View file
Open in desktop
Expand Up @@ -52,6 +52,17 @@ CommandLineOptions;
extern CommandLineOptions cl_options;


/* Information about command line entries that can not be stored in GOptionEntry. */
typedef struct
{
gboolean persistent; /* The option should be passed to "New Window" */
}
GeanyOptionEntryAux;

extern GOptionEntry optentries[];
extern GeanyOptionEntryAux optentries_aux[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very fond of exposing those outside of main, maybe it'd be nice to find a clean way to encapsulate access to those… any idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a function in libmain.c which could be called at argv parsing time and save a GPtrArray containing the argv to be forwarded into the already global GeanyApp structure. It would have the advantage of keeping all of the argv/GOption parsing code in the same file/place too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The argv parsing happens in GLib. A function may be executed at any time after that. We can't simply get them from the original argv.

I'd rather export a function returning the reproduced GPtrArray (or simply char **). This way, we can extend it reproduce different sets of options, should need be. And GeanyOptionEntryAux is trivial to extend...



typedef struct GeanyStatus
{
gboolean opening_session_files; /* state at startup while opening session files */
Expand Down
108 changes: 79 additions & 29 deletions src/utils.c
View file
Open in desktop
Expand Up @@ -32,6 +32,8 @@
#include "app.h"
#include "dialogs.h"
#include "document.h"
#include "keyfile.h"
#include "main.h"
#include "prefs.h"
#include "prefix.h"
#include "sciwrappers.h"
Expand Down Expand Up @@ -2094,6 +2096,7 @@ gchar *utils_get_user_config_dir(void)
}


#ifndef G_OS_WIN32
static gboolean is_osx_bundle(void)
{
#ifdef MAC_INTEGRATION
Expand All @@ -2106,6 +2109,7 @@ static gboolean is_osx_bundle(void)
#endif
return FALSE;
}
#endif


const gchar *utils_resource_dir(GeanyResourceDirType type)
Expand All @@ -2117,28 +2121,29 @@ const gchar *utils_resource_dir(GeanyResourceDirType type)
#ifdef G_OS_WIN32
gchar *prefix = win32_get_installation_dir();

resdirs[RESOURCE_DIR_PREFIX] = prefix;
resdirs[RESOURCE_DIR_DATA] = g_build_filename(prefix, "data", NULL);
resdirs[RESOURCE_DIR_ICON] = g_build_filename(prefix, "share", "icons", NULL);
resdirs[RESOURCE_DIR_DOC] = g_build_filename(prefix, "doc", NULL);
resdirs[RESOURCE_DIR_LOCALE] = g_build_filename(prefix, "share", "locale", NULL);
resdirs[RESOURCE_DIR_PLUGIN] = g_build_filename(prefix, "lib", "geany", NULL);
g_free(prefix);
#else
if (is_osx_bundle())
{
# ifdef MAC_INTEGRATION
gchar *prefix = gtkosx_application_get_resource_path();


resdirs[RESOURCE_DIR_PREFIX] = prefix;
resdirs[RESOURCE_DIR_DATA] = g_build_filename(prefix, "share", "geany", NULL);
resdirs[RESOURCE_DIR_ICON] = g_build_filename(prefix, "share", "icons", NULL);
resdirs[RESOURCE_DIR_DOC] = g_build_filename(prefix, "share", "doc", "geany", "html", NULL);
resdirs[RESOURCE_DIR_LOCALE] = g_build_filename(prefix, "share", "locale", NULL);
resdirs[RESOURCE_DIR_PLUGIN] = g_build_filename(prefix, "lib", "geany", NULL);
g_free(prefix);
# endif
}
else
{
resdirs[RESOURCE_DIR_PREFIX] = GEANY_PREFIX;
resdirs[RESOURCE_DIR_DATA] = g_build_filename(GEANY_DATADIR, "geany", NULL);
resdirs[RESOURCE_DIR_ICON] = g_build_filename(GEANY_DATADIR, "icons", NULL);
resdirs[RESOURCE_DIR_DOC] = g_build_filename(GEANY_DOCDIR, "html", NULL);
Expand All @@ -2152,39 +2157,84 @@ const gchar *utils_resource_dir(GeanyResourceDirType type)
}


/* "Reverse parse" GOptionEntry */
static gchar *utils_option_entry_reverse_parse(const GOptionEntry *optentry)
{
gchar *s = NULL;

switch (optentry->arg)
{
case G_OPTION_ARG_NONE:
{
gboolean val = *(gboolean *)optentry->arg_data;
gboolean reverse = (optentry->flags & G_OPTION_FLAG_REVERSE);
if ((val && !reverse) || (!val && reverse)) /* logical XOR */
s = g_strdup_printf("--%s", optentry->long_name);
}
break;

case G_OPTION_ARG_INT:
if (*(gint *)optentry->arg_data)
s = g_strdup_printf("--%s=%d", optentry->long_name, *(gint *)optentry->arg_data);
break;

case G_OPTION_ARG_STRING:
case G_OPTION_ARG_FILENAME:
if (*(gchar **)optentry->arg_data)
s = g_strdup_printf("--%s=%s", optentry->long_name, *(gchar **)optentry->arg_data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STRING and FILENAME have different encoding, I'm not sure if a conversion is needed.

GLib's docs say:

On UNIX systems, the argv that is passed to main() has no particular encoding, even to the extent that different parts of it may have different encodings. In general, normal arguments and flags will be in the current locale and filenames should be considered to be opaque byte strings. Proper use of G_OPTION_ARG_FILENAME vs G_OPTION_ARG_STRING is therefore important.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we have no STRING, so I'll discard it (the original SM option reproducer by Eugene supported even string and filename arrays, but I removed them at some point as unneeded).

As of the FILENAME, GLib says "returned in the GLib filename encoding", which is native for POSIX and UTF-8 under Windows. So a conversion is probably needed under Windows (that didn't matter for SM, which is *nix only). I'll test it to be sure.

break;

default:
g_warning("%s: %s: %d\n", G_STRFUNC, "Unsupported option entry type", optentry->arg);
}

return s;
}


void utils_start_new_geany_instance(const gchar *doc_path)
{
const gchar *command = is_osx_bundle() ? "open" : "geany";
gchar *exec_path = g_find_program_in_path(command);
gchar **argv;
int i, persistent;
int argc;
GError *error = NULL;

for (i = 0, persistent = 0; optentries[i].long_name; i++)
if (optentries_aux[i].persistent)
persistent++;

argv = g_new0(gchar *, persistent + 5); /* exectuable, -i, --, doc_path, NULL */
argv[0] = g_build_filename(utils_resource_dir(RESOURCE_DIR_PREFIX), "bin", "geany", NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This apparently is wrong on OSX, see #446 (comment) and b4n#1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. Since I don't want an unneeded RESOURCE_DIR_PREFIX under OSX, and g_build_filename() scattered in utils_resource_dir() and utils_start_new_geany_instance(), I'll do the following:

  • Rename utils_resource_dir() to utils_resource_path().
  • Rename RESOURCE_DIR_PREFIX to RESOURCE_FILE_GEANY or something (please suggest a name).
  • Alter utils_resource_path(RESOURCE_FILE_GEANY) to return gtkosx_application_get_executable_path() under OSX or prefix/bin/geany otherwise.

It is the executable name that we really need from utils_resource_path(), not the RESOURCE_DIR_PREFIX.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that win32 needs special casing as well, since the installer allows to install everywhere (I guess we have the code for that already)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils_resource_dir() already handles the windows "prefix".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, nevermind then


if (exec_path)
if (!g_file_test(argv[0], G_FILE_TEST_IS_EXECUTABLE))
{
GError *err = NULL;
const gchar *argv[6]; // max args + 1
gint argc = 0;
g_free(argv[0]);
argv[0] = g_strdup("geany");
}

argv[argc++] = exec_path;
if (is_osx_bundle())
{
argv[argc++] = "-n";
argv[argc++] = "-a";
argv[argc++] = "Geany";
argv[argc++] = doc_path;
}
else
for (i = 0, argc = 1; optentries[i].long_name; i++)
{
if (optentries_aux[i].persistent)
{
argv[argc++] = "-i";
argv[argc++] = doc_path;
}
argv[argc] = NULL;
char *option = utils_option_entry_reverse_parse(&optentries[i]);

if (!utils_spawn_async(NULL, (gchar**) argv, NULL, 0, NULL, NULL, NULL, &err))
{
g_printerr("Unable to open new window: %s", err->message);
g_error_free(err);
if (option)
argv[argc++] = option;
}
g_free(exec_path);
}
else
g_printerr("Unable to find 'geany'");

argv[argc++] = g_strdup("-i");
argv[argc++] = g_strdup("--");
argv[argc] = g_strdup(doc_path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for encoding, we need to make sure it's correct

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.


/* Ensure the configuration is saved to disk before launching the new instance */
configuration_save();

if (!spawn_async(NULL, NULL, argv, NULL, NULL, &error))
{
ui_set_statusbar(TRUE, _("Unable to open new Geany window: %s"), error->message);
g_error_free(error);
}

g_strfreev(argv);
}
1 change: 1 addition & 0 deletions src/utils.h
View file
Open in desktop
Expand Up @@ -202,6 +202,7 @@ const gchar *utils_find_open_xml_tag_pos(const gchar sel[], gint size);

typedef enum
{
RESOURCE_DIR_PREFIX,
RESOURCE_DIR_DATA,
RESOURCE_DIR_ICON,
RESOURCE_DIR_DOC,
Expand Down