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

avoid parsing opened files when loading project #1289 #1290

Merged
merged 1 commit into from
Nov 15, 2023
Merged

avoid parsing opened files when loading project #1289 #1290

merged 1 commit into from
Nov 15, 2023

Conversation

dolik-rce
Copy link
Contributor

Crude but working solution to #1289.

Disclaimer: I'm not really a C person, so any suggestion on how to improve the code quality are most welcome.

@dolik-rce dolik-rce mentioned this pull request Nov 3, 2023
Closed
dolik-rce
dolik-rce commented Nov 3, 2023
View reviewed changes
Copy link
Contributor Author

@dolik-rce dolik-rce left a comment

Choose a reason for hiding this comment

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

I just hope I didn't introduce any stupid memory leak. It's been years since I had to manage memory manually.

projectorganizer/src/prjorg-project.c Outdated Show resolved Hide resolved
projectorganizer/src/prjorg-project.c Outdated
@@ -497,13 +509,51 @@ gchar **prjorg_project_load_expanded_paths(GKeyFile * key_file)
}


static GPtrArray *get_session_files(GKeyFile *config)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mostly stolen from inspired by configuration_load_session_files in Geany. The only difference is that this one only reads the filename.

projectorganizer/src/prjorg-project.c Outdated Show resolved Hide resolved
@dolik-rce dolik-rce force-pushed the prjorg-do-not-parse-open-files branch 2 times, most recently from f8771f6 to 6cb68a7 Compare November 4, 2023 13:08
techee
techee requested changes Nov 14, 2023
View reviewed changes
Copy link
Member

@techee techee left a comment

Choose a reason for hiding this comment

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

@dolik-rce Thanks! I was hesitating whether this patch doesn't complicate things but it's not too bad so let's do that!

There are some minor comments in the review, nothing major, mostly some renames and minor cleanups.

projectorganizer/src/prjorg-project.c Outdated
@@ -532,6 +571,7 @@ void prjorg_project_open(GKeyFile * key_file)
generate_tag_prefs = utils_get_setting_integer(key_file, "prjorg", "generate_tag_prefs", PrjOrgTagAuto);
show_empty_dirs = utils_get_setting_boolean(key_file, "prjorg", "show_empty_dirs", TRUE);

open_files = get_session_files(key_file);
Copy link
Member

Choose a reason for hiding this comment

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

Big fat comment here what we are actually doing and why.

projectorganizer/src/prjorg-project.c Outdated
@@ -343,6 +345,7 @@ static void update_project(
gchar **header_patterns,
gchar **ignored_dirs_patterns,
gchar **ignored_file_patterns,
gchar **open_files,
Copy link
Member

Choose a reason for hiding this comment

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

session_files

projectorganizer/src/prjorg-project.c Outdated
static void regenerate_tags(PrjOrgRoot *root, gpointer user_data)
{
GHashTableIter iter;
gpointer key, value;
GPtrArray *source_files;
GHashTable *file_table;
gchar **open_files;
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest renaming it to session_files because these aren't open yet and may be confusing.

projectorganizer/src/prjorg-project.c Outdated
@@ -292,10 +293,11 @@ static void regenerate_tags(PrjOrgRoot *root, gpointer user_data)
gchar *utf8_path = key;
gchar *locale_path = utils_get_locale_from_utf8(utf8_path);
gchar *basename = g_path_get_basename(locale_path);
gboolean is_open = open_files && g_strv_contains(open_files, utf8_path);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it will_open.

Copy link
Member

Choose a reason for hiding this comment

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

Cast open_files to (const gchar **) to avoid warnings.

projectorganizer/src/prjorg-project.c Outdated
@@ -310,7 +312,7 @@ static void regenerate_tags(PrjOrgRoot *root, gpointer user_data)
}


void prjorg_project_rescan(void)
void prjorg_project_rescan(gchar **open_files)
Copy link
Member

Choose a reason for hiding this comment

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

Rename to session_files

Copy link
Member

Choose a reason for hiding this comment

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

In addition, I'd suggest converting it to a static function called rescan_project and having a separate prjorg_project_rescan() without any arguments and just calling rescan_project(NULL); inside it. This way you avoid the diffs adding the NULL parameter across the project.

projectorganizer/src/prjorg-project.c Outdated
void prjorg_project_open(GKeyFile * key_file)
{
gchar **source_patterns, **header_patterns, **ignored_dirs_patterns, **ignored_file_patterns, **external_dirs, **dir_ptr, *last_name;
gint generate_tag_prefs;
gboolean show_empty_dirs;
GSList *elem = NULL, *ext_list = NULL;
gchar *utf8_base_path;
GPtrArray *open_files;
Copy link
Member

Choose a reason for hiding this comment

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

session_files

Copy link
Member

Choose a reason for hiding this comment

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

In addition, with the suggested change in get_session_files, change to gchar **.

projectorganizer/src/prjorg-project.c Outdated
@@ -275,14 +275,15 @@ static GeanyFiletype *filetypes_detect(const gchar *utf8_filename)
return ft;
}


Copy link
Member

Choose a reason for hiding this comment

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

keep two empty lines before functions

projectorganizer/src/prjorg-project.c Outdated
}
g_ptr_array_add(files, NULL);

return files;
Copy link
Member

Choose a reason for hiding this comment

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

change to return g_ptr_array_free(files, FALSE); and return gchar ** from the function.

projectorganizer/src/prjorg-project.c Outdated
@@ -563,6 +604,7 @@ void prjorg_project_open(GKeyFile * key_file)
g_strfreev(ignored_dirs_patterns);
g_strfreev(ignored_file_patterns);
g_strfreev(external_dirs);
g_ptr_array_free(open_files, TRUE);
Copy link
Member

Choose a reason for hiding this comment

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

Use g_strfreev with the other changes above.

@dolik-rce dolik-rce force-pushed the prjorg-do-not-parse-open-files branch from 6cb68a7 to a8c8ca2 Compare November 14, 2023 18:25
@dolik-rce
Copy link
Contributor Author

I was hesitating whether this patch doesn't complicate things but it's not too bad so let's do that!

It is even less complicated now, after I applied your suggestions :-)

@techee techee merged commit 6227d2b into geany:master Nov 15, 2023
2 checks passed
@techee
Copy link
Member

techee commented Nov 15, 2023

Thanks! Merged now.

@dolik-rce dolik-rce mentioned this pull request Dec 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rdipardo rdipardo rdipardo left review comments

@techee techee techee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dolik-rce @techee @rdipardo