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.

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

build.c: refactor set_stop_button() and add_menu_accel() #2271

Open
wants to merge 4 commits into
base: master
from

Conversation

@ntrel
Copy link
Member

commented Aug 23, 2019

Also fix unused variable warning.

Please see commits for details, each commit is unrelated but I thought it best to group these small changes together to save having 3 PRs with only simple changes in.

@@ -837,16 +837,17 @@ static gchar *prepare_run_cmd(GeanyDocument *doc, gchar **working_dir, guint cmd
run_cmd = g_strdup_printf("\"%s\" \"%s\" %d %s", helper, *working_dir, autoclose ? 1 : 0, cmd_string);
g_free(helper);
#else
GError *error = NULL;

This comment has been minimized.

Copy link
@ntrel

ntrel Aug 23, 2019

Author Member

This was causing an unused variable warning on Windows.

src/build.c Outdated Show resolved Hide resolved
src/build.c Outdated
@@ -1547,25 +1542,17 @@ static void set_stop_button(gboolean stop)
GtkToolButton *run_button;

run_button = GTK_TOOL_BUTTON(toolbar_get_widget_by_name("Run"));
if (run_button != NULL)
button_stock_id = gtk_tool_button_get_stock_id(run_button);
if (!run_button) return;

This comment has been minimized.

Copy link
@codebrainz

codebrainz Aug 24, 2019

Member

Minor nit, but should have space after ! and line break before return.

This comment has been minimized.

Copy link
@ntrel

ntrel Aug 29, 2019

Author Member

@codebrainz Is this documented in HACKING?

This comment has been minimized.

Copy link
@codebrainz

codebrainz Aug 29, 2019

Member

More or less: Line break, and operator spacing (although it says two operand) but also fit in with existing code, for example here and here in nearby code (counter example here)

This comment has been minimized.

Copy link
@ntrel

ntrel Sep 1, 2019

Author Member

I've added the line break to comply with HACKING. There are 3 existing if statements in build.c with a return clause on the same line, but I see that there are not many of these in src/.

space after !

I haven't done this, there are 16 matches for the regex \bif \(!\S in build.c.

@ntrel ntrel force-pushed the ntrel:build-code branch from a53ccbd to a561eb1 Sep 1, 2019

@ntrel ntrel force-pushed the ntrel:build-code branch from a561eb1 to 2682f30 Sep 1, 2019

@ntrel ntrel changed the title build.c: Fix memory leak & make some small refactorings build.c: refactor set_stop_button() and add_menu_accel() Sep 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.