Skip to content
GitLab
Commit 4a5b7377 authored by Vanadiae's avatar Vanadiae
Browse files

wip: show only todos for added lines 

TBD: handle untracked files, finding merge-base (big rabbit-hole), make sure
both staged and unstaged changes are handled
parent f086e66d
Pipeline #361100 failed with stage
in 25 minutes and 47 seconds
Hide whitespace changes
Inline Side-by-side
src/plugins/todo/gbp-todo-model.c
View file @ 4a5b7377
......@@ -42,6 +42,7 @@
struct _GbpTodoModel {
GtkListStore parent_instance;
IdeVcs *vcs;
gboolean show_only_added_lines;
};
typedef struct
......@@ -290,6 +291,7 @@ gbp_todo_model_init (GbpTodoModel *self)
gtk_list_store_set_column_types (GTK_LIST_STORE (self),
G_N_ELEMENTS (column_types),
column_types);
self->show_only_added_lines = TRUE;
}
/**
......@@ -311,49 +313,9 @@ gbp_todo_model_new (IdeVcs *vcs)
}
static void
gbp_todo_model_mine_worker (IdeTask *task,
gpointer source_object,
gpointer task_data,
GCancellable *cancellable)
setup_grep_launcher (IdeSubprocessLauncher *launcher,
Mine *m)
{
g_autoptr(IdeSubprocessLauncher) launcher = NULL;
g_autoptr(IdeSubprocess) subprocess = NULL;
g_autoptr(GError) error = NULL;
g_autoptr(GPtrArray) items = NULL;
g_autoptr(GbpTodoItem) item = NULL;
g_autoptr(GBytes) bytes = NULL;
g_autoptr(GTimer) timer = g_timer_new ();
g_autofree gchar *workpath = NULL;
GbpTodoModel *self = source_object;
Mine *m = task_data;
IdeLineReader reader;
ResultInfo *info;
gchar *stdoutstr = NULL;
gchar *line;
gsize pathlen = 0;
gsize stdoutstr_len;
gsize len;
g_assert (IDE_IS_TASK (task));
g_assert (GBP_IS_TODO_MODEL (self));
g_assert (m != NULL);
g_assert (G_IS_FILE (m->file));
g_assert (!cancellable || G_IS_CANCELLABLE (cancellable));
launcher = ide_subprocess_launcher_new (G_SUBPROCESS_FLAGS_STDOUT_PIPE);
if (!(workpath = g_file_get_path (m->workdir)))
{
ide_task_return_new_error (task,
G_IO_ERROR,
G_IO_ERROR_NOT_SUPPORTED,
"Cannot run on non-native file-systems");
return;
}
pathlen = strlen (workpath);
ide_subprocess_launcher_set_cwd (launcher, workpath);
if (m->use_git_grep)
{
ide_subprocess_launcher_push_argv (launcher, "git");
......@@ -423,48 +385,146 @@ gbp_todo_model_mine_worker (IdeTask *task,
ide_subprocess_launcher_push_argv (launcher, "^.{0,256}$");
}
}
}
if (g_file_query_file_type (m->file, 0, NULL) != G_FILE_TYPE_DIRECTORY)
static void
process_diff_output (GbpTodoModel *self,
GPtrArray *items,
IdeLineReader *reader,
GBytes *bytes,
Mine *m)
{
char *line;
gsize len;
gboolean is_header = TRUE;
const char *filepath = NULL;
/* "real" means in-tree file here, not line number in the diff file… This is set
* when we reach a hunk's @@ delimited range header, and we increment it on each
* subsequent diff lines.
*/
guint current_real_lineno;
while (NULL != (line = ide_line_reader_next (reader, &len)))
{
g_autofree gchar *path = NULL;
if (len == 0)
continue;
line[len] = '\0';
path = g_file_get_path (m->workdir);
ide_subprocess_launcher_push_argv (launcher, path);
}
if (is_header)
{
/* This is_header variable is necessary because +++ is used in the header
* to indicate the changed file path, but + alone is used in the diff part
* and if we didn't have is_header it would break when there is a diff
* file laying around in the tree (as in that case we could legitimately have
* +++ in the diff part of the patch). So the range indicator @ char is what
* we'll use to separate these out.
*/
if (line[0] == '@')
is_header = FALSE;
else if (g_str_has_prefix (line, "+++ "))
{
char *timestamp_tab;
/* Spawn our grep process */
if (NULL == (subprocess = ide_subprocess_launcher_spawn (launcher, cancellable, &error)))
{
ide_task_return_error (task, g_steal_pointer (&error));
return;
}
filepath = line + strlen ("+++ ");
/* Unified diffs in POSIX allow extra things after file, like timestamp.
* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07
*/
timestamp_tab = strchr (filepath, '\t');
if (timestamp_tab)
*timestamp_tab = '\0';
/* Read all of the output into a giant string */
if (!ide_subprocess_communicate_utf8 (subprocess, NULL, NULL, &stdoutstr, NULL, &error))
{
ide_task_return_error (task, g_steal_pointer (&error));
return;
}
/* This is what git uses as filename after the +++ when the file gets deleted.
* In that case we won't have any "+" diff added lines for this hunk, only
* "-" deleted ones, so we don't risk anything.
*/
if (g_strcmp0 (filepath, "/dev/null") == 0)
filepath = NULL;
else
{
g_assert (g_str_has_prefix (filepath, "b/"));
filepath += strlen ("b/");
}
}
}
/*
* To avoid lots of string allocations in the model, we instead
* store GObjects which contain a reference to the whole buffer
* (the GBytes) and raw pointers into that data. We'll create
* the buffer up front, but still mutate the contents as we
* walk through it.
*/
stdoutstr_len = strlen (stdoutstr);
bytes = g_bytes_new_take (stdoutstr, stdoutstr_len);
if (!is_header)
{
switch (line[0])
{
case '@':
{
char *range, *range_end, *comma;
g_assert (filepath != NULL);
/* Hunk ranges format is according to POSIX's diff:
* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07
*/
g_assert (g_str_has_prefix (line, "@@ "));
range = line + strlen("@@ ");
/* Ignore the deleted lines range. */
range = strchr(range, ' ');
g_assert (range != NULL);
range += strlen (" ");
g_assert (range[0] == '+');
range += strlen("+");
g_assert (g_ascii_isdigit (*range));
range_end = strstr (range, " @@");
g_assert (range_end != NULL);
*range_end = '\0';
comma = strchr (range, ',');
/* The comma is not mandatory in POSIX, as in that case the number
* of lines is assumed to be 1, and in which case we'll have our NUL
* byte set at the right spot anyway with the end " @@".
*/
if (comma)
*comma = '\0';
current_real_lineno = g_ascii_strtoull (range, NULL, 10);
/* For the time being we ignore the lines count part */
/* lines_count = g_ascii_strtoull (comma + 1, NULL, 10); */
break;
}
case '+':
for (guint i = 0; i < G_N_ELEMENTS (keywords); i++) {
const char *real_line = line + strlen ("+");
if (strstr (real_line, keywords[i]) != NULL)
{
/* We found a todo line, so create the item and add it straight away. */
GbpTodoItem *item = gbp_todo_item_new (bytes);
gbp_todo_item_set_path (item, filepath);
gbp_todo_item_set_lineno (item, current_real_lineno);
gbp_todo_item_add_line (item, real_line);
g_ptr_array_add (items, item);
/* This can happen if someone uses e.g. FIXME TODO on the same line, in
* which case there's no point in having to identical todo items here.
*/
break;
}
}
current_real_lineno += 1;
break;
default: /* We only care about added lines, not the rest. */
current_real_lineno += 1;
break;
}
}
}
}
items = g_ptr_array_new_with_free_func (g_object_unref);
static void
process_grep_output (GbpTodoModel *self,
GPtrArray *items,
IdeLineReader *reader,
GBytes *bytes,
Mine *m,
const char *workpath)
{
g_autoptr(GbpTodoItem) item = NULL;
gchar *line;
gsize pathlen = strlen (workpath);
gsize len;
/*
* Process the STDOUT string iteratively, while trying to be tidy
* with additional allocations. If we overwrite the trailing \n to
* a \0, we can use it as a C string.
*/
ide_line_reader_init (&reader, stdoutstr, stdoutstr_len);
while (NULL != (line = ide_line_reader_next (&reader, &len)))
while (NULL != (line = ide_line_reader_next (reader, &len)))
{
g_autoptr(GMatchInfo) match_info1 = NULL;
g_autoptr(GMatchInfo) match_info2 = NULL;
......@@ -599,6 +659,127 @@ gbp_todo_model_mine_worker (IdeTask *task,
g_clear_object (&item);
}
}
}
static void
gbp_todo_model_mine_worker (IdeTask *task,
gpointer source_object,
gpointer task_data,
GCancellable *cancellable)
{
g_autoptr(IdeSubprocessLauncher) launcher = NULL;
g_autoptr(IdeSubprocess) subprocess = NULL;
g_autoptr(GError) error = NULL;
g_autoptr(GPtrArray) items = NULL;
g_autoptr(GBytes) bytes = NULL;
g_autoptr(GTimer) timer = g_timer_new ();
g_autofree gchar *workpath = NULL;
GbpTodoModel *self = source_object;
Mine *m = task_data;
IdeLineReader reader;
ResultInfo *info;
gchar *stdoutstr = NULL;
gsize stdoutstr_len;
g_assert (IDE_IS_TASK (task));
g_assert (GBP_IS_TODO_MODEL (self));
g_assert (m != NULL);
g_assert (G_IS_FILE (m->file));
g_assert (!cancellable || G_IS_CANCELLABLE (cancellable));
launcher = ide_subprocess_launcher_new (G_SUBPROCESS_FLAGS_STDOUT_PIPE);
if (!(workpath = g_file_get_path (m->workdir)))
{
ide_task_return_new_error (task,
G_IO_ERROR,
G_IO_ERROR_NOT_SUPPORTED,
"Cannot run on non-native file-systems");
return;
}
/* It means we're doing a from-scratch mining, in which case we'll want to
* have a fresh start.
*/
if (g_file_equal (m->file, m->workdir))
gtk_list_store_clear (GTK_LIST_STORE (self));
ide_subprocess_launcher_set_cwd (launcher, workpath);
if (m->use_git_grep && self->show_only_added_lines)
{
ide_subprocess_launcher_push_argv (launcher, "git");
ide_subprocess_launcher_push_argv (launcher, "diff");
ide_subprocess_launcher_push_argv (launcher, "--unified=0");
/* TODO: this means we only look for non committed TODOs. Ideally we'd look for any TODOs
* between the merge-base of the branch and HEAD. Not sure how to get that merge-base properly
* though with proper heuristic. Wil need to have a closer look at it. It seems like it's stored
* in .git/config, but it's not really a good idea to parse that…
*/
/* FIXME: Due to the way git works, it's not straight-forward to include untracked
* files in the diff without adding signifiant code/complexity. git diff /dev/null untracked_file
* does diff the way we'd like here, but we need to manually run over each untracked file
* (which we hence need to first collect from whatever git command, e.g. "git status -u --porcelain").
* It would kinda be confusing not to show those untracked files, so would be nice to solve
* that either way.
*/
ide_subprocess_launcher_push_argv (launcher, "HEAD" "~3"); /* TODO: remove test ~3 */
}
else
{
setup_grep_launcher (launcher, m);
}
/* FIXME: This code here is wrong I think. To me it means that when we know it's a single
* file change (i.e. saved buffer), we grep on the whole project which is overkill and not
* desired. And when m->file is a folder, it doesn't pass any path to grep which means it
* will here too scan the whole project instead of just this particular directory. Both
* of these behaviors conflict with the documentation and just the expected behavior really.
*/
if (g_file_query_file_type (m->file, 0, NULL) != G_FILE_TYPE_DIRECTORY)
{
g_autofree gchar *path = NULL;
path = g_file_get_path (m->workdir);
ide_subprocess_launcher_push_argv (launcher, path);
}
/* Spawn our process */
if (NULL == (subprocess = ide_subprocess_launcher_spawn (launcher, cancellable, &error)))
{
ide_task_return_error (task, g_steal_pointer (&error));
return;
}
/* Read all of the output into a giant string */
if (!ide_subprocess_communicate_utf8 (subprocess, NULL, NULL, &stdoutstr, NULL, &error))
{
ide_task_return_error (task, g_steal_pointer (&error));
return;
}
/*
* To avoid lots of string allocations in the model, we instead
* store GObjects which contain a reference to the whole buffer
* (the GBytes) and raw pointers into that data. We'll create
* the buffer up front, but still mutate the contents as we
* walk through it.
*/
stdoutstr_len = strlen (stdoutstr);
bytes = g_bytes_new_take (stdoutstr, stdoutstr_len);
items = g_ptr_array_new_with_free_func (g_object_unref);
/*
* Process the STDOUT string iteratively, while trying to be tidy
* with additional allocations. If we overwrite the trailing \n to
* a \0, we can use it as a C string.
*/
ide_line_reader_init (&reader, stdoutstr, stdoutstr_len);
if (m->use_git_grep && self->show_only_added_lines)
process_diff_output (self, items, &reader, bytes, m);
else
process_grep_output (self, items, &reader, bytes, m, workpath);
g_debug ("Located %u TODO items in %0.4lf seconds",
items->len, g_timer_elapsed (timer, NULL));
......@@ -674,7 +855,7 @@ gbp_todo_model_mine_async (GbpTodoModel *self,
m = g_slice_new0 (Mine);
m->file = g_object_ref (file);
m->workdir = g_object_ref (workdir);
m->use_git_grep = is_typed (self->vcs, "IdeGitVcs");
m->use_git_grep = is_typed (self->vcs, "GbpGitVcs");
ide_task_set_task_data (task, m, mine_free);
ide_task_run_in_thread (task, gbp_todo_model_mine_worker);
......@@ -702,3 +883,20 @@ gbp_todo_model_mine_finish (GbpTodoModel *self,
return ide_task_propagate_boolean (IDE_TASK (result), error);
}
/**
* gbp_todo_model_set_show_only_added_lines:
* @self: a #GbpTodoModel
* @show_only_changed_lines: whether to show only added TODO lines
*
* Sets whether later mining operations will only look for TODOs lines
* marked as added by the VCS.
*
* Since: 42.0
*/
void
gbp_todo_model_set_show_only_added_lines (GbpTodoModel *self,
gboolean show_only_added_lines)
{
self->show_only_added_lines = show_only_added_lines;
}
src/plugins/todo/gbp-todo-model.h
View file @ 4a5b7377
......@@ -29,14 +29,16 @@ G_BEGIN_DECLS
G_DECLARE_FINAL_TYPE (GbpTodoModel, gbp_todo_model, GBP, TODO_MODEL, GtkListStore)
GbpTodoModel *gbp_todo_model_new (IdeVcs *vcs);
void gbp_todo_model_mine_async (GbpTodoModel *self,
GFile *file,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data);
gboolean gbp_todo_model_mine_finish (GbpTodoModel *self,
GAsyncResult *result,
GError **error);
GbpTodoModel *gbp_todo_model_new (IdeVcs *vcs);
void gbp_todo_model_mine_async (GbpTodoModel *self,
GFile *file,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data);
gboolean gbp_todo_model_mine_finish (GbpTodoModel *self,
GAsyncResult *result,
GError **error);
void gbp_todo_model_set_show_only_added_lines (GbpTodoModel *self,
gboolean show_only_added_lines);
G_END_DECLS
src/plugins/todo/gbp-todo-panel.c
View file @ 4a5b7377
......@@ -29,12 +29,13 @@
struct _GbpTodoPanel
{
DzlDockWidget parent_instance;
DzlDockWidget parent_instance;
GbpTodoModel *model;
GbpTodoModel *model;
GtkTreeView *tree_view;
GtkStack *stack;
GtkTreeView *tree_view;
GtkStack *stack;
GtkCheckButton *only_added_lines_checkbox;
};
G_DEFINE_FINAL_TYPE (GbpTodoPanel, gbp_todo_panel, DZL_TYPE_DOCK_WIDGET)
......@@ -47,6 +48,13 @@ enum {
static GParamSpec *properties [N_PROPS];
enum {
MUST_MINE_AGAIN,
LAST_SIGNAL
};
static guint signals [LAST_SIGNAL];
static void
gbp_todo_panel_cell_data_func (GtkCellLayout *cell_layout,
GtkCellRenderer *cell,
......@@ -206,6 +214,16 @@ gbp_todo_panel_query_tooltip (GbpTodoPanel *self,
return FALSE;
}
static void
on_checkbox_toggled_cb (GtkToggleButton *togglebutton,
gpointer user_data)
{
GbpTodoPanel *self = GBP_TODO_PANEL (user_data);
gbp_todo_model_set_show_only_added_lines (self->model, gtk_toggle_button_get_active (togglebutton));
g_signal_emit (self, signals[MUST_MINE_AGAIN], 0);
}
static void
gbp_todo_panel_destroy (GtkWidget *widget)
{
......@@ -270,6 +288,14 @@ gbp_todo_panel_class_init (GbpTodoPanelClass *klass)
widget_class->destroy = gbp_todo_panel_destroy;
signals [MUST_MINE_AGAIN] = g_signal_new ("must-mine-again",
G_TYPE_FROM_CLASS (klass),
G_SIGNAL_RUN_LAST,
0,
NULL, NULL, NULL,
G_TYPE_NONE,
0);
properties [PROP_MODEL] =
g_param_spec_object ("model",
"Model",
......@@ -286,6 +312,7 @@ gbp_todo_panel_init (GbpTodoPanel *self)
GtkWidget *scroller;
GtkWidget *empty;
GtkTreeSelection *selection;
GtkWidget *vbox;
self->stack = g_object_new (GTK_TYPE_STACK,
"transition-duration", 333,
......@@ -304,13 +331,24 @@ gbp_todo_panel_init (GbpTodoPanel *self)
NULL);
gtk_container_add (GTK_CONTAINER (self->stack), empty);
vbox = gtk_box_new (GTK_ORIENTATION_VERTICAL, 6);
gtk_widget_show (vbox);
gtk_container_add_with_properties (GTK_CONTAINER (self->stack), vbox,
"name", "todos",
NULL);
self->only_added_lines_checkbox = GTK_CHECK_BUTTON (gtk_check_button_new_with_mnemonic (_("_Only show TODOs on changed lines")));
gtk_widget_show (GTK_WIDGET (self->only_added_lines_checkbox));
gtk_container_add (GTK_CONTAINER (vbox), GTK_WIDGET (self->only_added_lines_checkbox));
/* Showing only the TODOs that _you_ added is far more useful than seeing the
* TODOs in the entire project.
*/
gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (self->only_added_lines_checkbox), TRUE);
g_signal_connect (self->only_added_lines_checkbox, "toggled", G_CALLBACK (on_checkbox_toggled_cb), self);
scroller = g_object_new (GTK_TYPE_SCROLLED_WINDOW,
"visible", TRUE,
"vexpand", TRUE,
NULL);
gtk_container_add_with_properties (GTK_CONTAINER (self->stack), scroller,
"name", "todos",
NULL);
gtk_container_add (GTK_CONTAINER (vbox), scroller);
self->tree_view = g_object_new (IDE_TYPE_FANCY_TREE_VIEW,
"has-tooltip", TRUE,
......@@ -366,7 +404,11 @@ gbp_todo_panel_set_model (GbpTodoPanel *self,
if (g_set_object (&self->model, model))
{
if (self->model != NULL)
gtk_tree_view_set_model (self->tree_view, GTK_TREE_MODEL (self->model));
{
gtk_tree_view_set_model (self->tree_view, GTK_TREE_MODEL (self->model));
gbp_todo_model_set_show_only_added_lines (self->model,
gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (self->only_added_lines_checkbox)));
}
else
gtk_tree_view_set_model (self->tree_view, NULL);
......
src/plugins/todo/gbp-todo-workspace-addin.c
View file @ 4a5b7377
......@@ -62,6 +62,21 @@ gbp_todo_workspace_addin_mine_cb (GObject *object,
gbp_todo_panel_make_ready (self->panel);
}
static void
gbp_todo_workspace_addin_must_mine_again_cb (GbpTodoWorkspaceAddin *self,
GbpTodoPanel *panel)
{
g_assert (GBP_IS_TODO_WORKSPACE_ADDIN (self));
g_assert (GBP_IS_TODO_PANEL (panel));
/* FIXME: must cancel cancellagle, and add a spinner (esp. needed for the non diff mode) */
gbp_todo_model_mine_async (self->model,
self->workdir,
self->cancellable,
gbp_todo_workspace_addin_mine_cb,
g_object_ref (self));
}
static void
gbp_todo_workspace_addin_presented_cb (GbpTodoWorkspaceAddin *self,
GbpTodoPanel *panel)
......@@ -148,6 +163,11 @@ gbp_todo_workspace_addin_load (IdeWorkspaceAddin *addin,
G_CALLBACK (gbp_todo_workspace_addin_presented_cb),
self,
G_CONNECT_SWAPPED);
g_signal_connect_object (self->panel,
"must-mine-again",
G_CALLBACK (gbp_todo_workspace_addin_must_mine_again_cb),
self,
G_CONNECT_SWAPPED);
g_signal_connect (self->panel,
"destroy",
G_CALLBACK (gtk_widget_destroyed),
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment