Skip to content

Commit

Permalink
do not recreate Row # if it is already first column in file (#288)
Browse files Browse the repository at this point in the history
Sheet:
* do not recreate Row # if it is already first column in file
* fix memory leaks
* fix index crash if interrupted before complete
* fix nested filters not being applied cumulatively
* retain original row numbers in filters; update tests
* add tests
  • Loading branch information
liquidaty authored Nov 14, 2024
1 parent d567df2 commit 5dfe914
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 53 deletions.
1 change: 1 addition & 0 deletions app/2db.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *zs
struct zsv_prop_handler *custom_prop_handler, const char *opts_used) {
(void)(zsv_opts);
(void)(opts_used);
(void)(custom_prop_handler);
FILE *f_in = NULL;
int err = 0;
struct zsv_2db_options opts = {0};
Expand Down
37 changes: 27 additions & 10 deletions app/sheet.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ struct zsvsheet_opts {
#define ZSVSHEET_CELL_DISPLAY_MIN_WIDTH 10
static size_t zsvsheet_cell_display_width(struct zsvsheet_ui_buffer *ui_buffer,
struct zsvsheet_display_dimensions *ddims) {
size_t width = ddims->columns / (ui_buffer->dimensions.col_count + (ui_buffer->rownum_col_offset ? 1 : 0));
size_t width = ddims->columns /
(ui_buffer->dimensions.col_count + (ui_buffer->rownum_col_offset && !ui_buffer->has_row_num ? 1 : 0));
return width < ZSVSHEET_CELL_DISPLAY_MIN_WIDTH ? ZSVSHEET_CELL_DISPLAY_MIN_WIDTH : width;
}

Expand Down Expand Up @@ -412,15 +413,16 @@ static zsvsheet_status zsvsheet_filter_handler(struct zsvsheet_proc_context *ctx
int prompt_footer_row = (int)(di->dimensions->rows - di->dimensions->footer_span);
int err;

UNUSED(ctx);

get_subcommand("Filter", prompt_buffer, sizeof(prompt_buffer), prompt_footer_row);
if (*prompt_buffer == '\0')
goto no_input;

if ((err = zsvsheet_ui_buffer_open_file(current_ui_buffer->filename, &current_ui_buffer->zsv_opts, prompt_buffer,
state->custom_prop_handler, state->opts_used, di->ui_buffers.base,
di->ui_buffers.current))) {
const char *data_filename = zsvsheet_buffer_data_filename(current_ui_buffer);
char is_filtered_file = !(data_filename == current_ui_buffer->filename);
struct zsv_opts *zsv_opts = is_filtered_file ? NULL : &current_ui_buffer->zsv_opts;
const char *opts_used = is_filtered_file ? NULL : state->opts_used;
if ((err = zsvsheet_ui_buffer_open_file(data_filename, zsv_opts, prompt_buffer, state->custom_prop_handler, opts_used,
di->ui_buffers.base, di->ui_buffers.current))) {
if (err > 0)
zsvsheet_priv_set_status(di->dimensions, 1, "%s: %s", current_ui_buffer->filename, strerror(err));
else if (err < 0)
Expand All @@ -429,9 +431,18 @@ static zsvsheet_status zsvsheet_filter_handler(struct zsvsheet_proc_context *ctx
zsvsheet_priv_set_status(di->dimensions, 1, "Not found: %s", prompt_buffer);
return zsvsheet_status_ignore;
}
if (current_ui_buffer->dimensions.row_count < 2) {

struct zsvsheet_ui_buffer *new_ui_buffer = *state->display_info.ui_buffers.current;
/*
if (new_ui_buffer->dimensions.row_count < 2) {
zsvsheet_ui_buffer_pop(di->ui_buffers.base, di->ui_buffers.current, NULL);
zsvsheet_priv_set_status(di->dimensions, 1, "Not found: %s", prompt_buffer);
return zsvsheet_status_ignore;
}
*/
if (is_filtered_file) {
free(new_ui_buffer->filename);
new_ui_buffer->filename = strdup(current_ui_buffer->filename);
}
no_input:
return zsvsheet_status_ok;
Expand All @@ -446,6 +457,8 @@ zsvsheet_status zsvsheet_builtin_proc_handler(struct zsvsheet_proc_context *ctx)

switch (ctx->proc_id) {
case zsvsheet_builtin_proc_quit:
// while(*state->display_info.ui_buffers.current)
// zsvsheet_ui_buffer_pop(state->display_info.ui_buffers.base, state->display_info.ui_buffers.current, NULL);
return zsvsheet_status_exit;
case zsvsheet_builtin_proc_resize:
*(state->display_info.dimensions) = get_display_dimensions(1, 1);
Expand Down Expand Up @@ -573,10 +586,13 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
perror(filename);
else
fprintf(stderr, "%s: no data found", filename); // to do: change this to a base-buff status msg
return -1;

err = -1;
goto zsvsheet_exit;
}
}

err = 0;
header_span = 1;
initscr();
noecho();
Expand Down Expand Up @@ -650,9 +666,10 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op

endwin();
free(handler_state.find);
zsvsheet_ui_buffers_delete(ui_buffers);
zsvsheet_exit:
zsvsheet_ui_buffers_delete(current_ui_buffer);
zsvsheet_key_handlers_delete(&zsvsheet_key_handlers, &zsvsheet_next_key_handler);
return 0;
return err;
}

const char *display_cell(struct zsvsheet_screen_buffer *buff, size_t data_row, size_t data_col, int row, int col,
Expand Down
2 changes: 2 additions & 0 deletions app/sheet/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ int zsvsheet_ui_buffer_open_file(const char *filename, const struct zsv_opts *zs
int err = 0;
struct zsvsheet_ui_buffer *tmp_ui_buffer = NULL;
uibopts.row_filter = row_filter;
if (!opts_used)
opts_used = "";
if ((err = read_data(&tmp_ui_buffer, &uibopts, 0, 0, 0, &zsvsheet_opts, custom_prop_handler, opts_used)) != 0 ||
!tmp_ui_buffer || !tmp_ui_buffer->buff_used_rows) {
zsvsheet_ui_buffer_delete(tmp_ui_buffer);
Expand Down
21 changes: 16 additions & 5 deletions app/sheet/index.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,34 @@ static void save_filtered_file_row_handler(void *ctx) {
struct zsvsheet_indexer *ixr = ctx;
zsv_parser parser = ixr->parser;
size_t col_count = zsv_cell_count(parser);

ixr->row_num++;
if (col_count == 0)
return;

struct zsv_cell first_cell = zsv_get_cell(parser, 0);
if (ixr->seen_header) {
struct zsv_cell first_cell = zsv_get_cell(parser, 0);
struct zsv_cell last_cell = zsv_get_cell(parser, col_count - 1);

if (!memmem(first_cell.str, last_cell.str - first_cell.str + last_cell.len, ixr->filter, ixr->filter_len))
return;
// future enhancement: optionally, handle if row may have unusual quotes e.g. cell1,"ce"ll2,cell3
} else {
ixr->seen_header = 1;
if (first_cell.len == ZSVSHEET_ROWNUM_HEADER_LEN && !memcmp(first_cell.str, ZSVSHEET_ROWNUM_HEADER, first_cell.len))
ixr->has_row_num = 1;
}

char row_started = 0;
if (!ixr->has_row_num) {
// create our own rownum column
row_started = 1;
if (ixr->row_num == 1)
zsv_writer_cell_s(ixr->writer, 1, (const unsigned char *)"Row #", 0); // to do: consolidate "Row #"
else
zsv_writer_cell_zu(ixr->writer, 1, ixr->row_num - 1);
}
for (size_t i = 0; i < col_count; i++) {
struct zsv_cell cell = zsv_get_cell(parser, i);
zsv_writer_cell(ixr->writer, i == 0, cell.str, cell.len, cell.quoted);
zsv_writer_cell(ixr->writer, i == 0 && row_started == 0, cell.str, cell.len, cell.quoted);
}
}

Expand Down Expand Up @@ -119,7 +130,7 @@ enum zsv_index_status build_memory_index(struct zsvsheet_index_opts *optsp) {
ret = zsv_index_status_ok;
*optsp->index = ixr.ix;
} else
free(ixr.ix);
zsv_index_delete(ixr.ix);

out:
zsv_delete(ixr.parser);
Expand Down
5 changes: 4 additions & 1 deletion app/sheet/index.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ struct zsvsheet_indexer {
size_t filter_len;
zsv_csv_writer writer;
FILE *filter_stream;
char seen_header;
size_t row_num; // 1-based row number (1 = header row, 2 = first data row)
unsigned char seen_header : 1;
unsigned char has_row_num : 1;
unsigned char _ : 6;
};

struct zsvsheet_index_opts {
Expand Down
63 changes: 37 additions & 26 deletions app/sheet/read-data.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,20 @@ static void *get_data_index(void *d);
static void get_data_index_async(struct zsvsheet_ui_buffer *uibuffp, const char *filename, struct zsv_opts *optsp,
const char *row_filter, struct zsv_prop_handler *custom_prop_handler,
const char *opts_used, pthread_mutex_t *mutexp) {
struct zsvsheet_index_opts *gdi = calloc(1, sizeof(*gdi));
gdi->mutexp = mutexp;
gdi->filename = filename;
gdi->data_filenamep = &uibuffp->data_filename;
gdi->zsv_opts = *optsp;
gdi->row_filter = row_filter;
gdi->index = &uibuffp->index;
gdi->index_ready = &uibuffp->index_ready;
gdi->custom_prop_handler = custom_prop_handler;
gdi->opts_used = opts_used;
gdi->uib = uibuffp;
struct zsvsheet_index_opts *ixopts = calloc(1, sizeof(*ixopts));
ixopts->mutexp = mutexp;
ixopts->filename = filename;
ixopts->data_filenamep = &uibuffp->data_filename;
ixopts->zsv_opts = *optsp;
ixopts->row_filter = row_filter;
ixopts->index = &uibuffp->index;
ixopts->index_ready = &uibuffp->index_ready;
ixopts->custom_prop_handler = custom_prop_handler;
ixopts->opts_used = opts_used;
ixopts->uib = uibuffp;
ixopts->uib->ixopts = ixopts;
pthread_t thread;
pthread_create(&thread, NULL, get_data_index, gdi);
pthread_create(&thread, NULL, get_data_index, ixopts);
pthread_detach(thread);
}

Expand Down Expand Up @@ -101,6 +102,7 @@ static int read_data(struct zsvsheet_ui_buffer **uibufferp, // a new zsvsheet_
filter_opts.max_row_size = opts.max_row_size;
filter_opts.max_rows = opts.max_rows;
opts = filter_opts;
uibuff->has_row_num = 1; // move this to coincide with when data_filename is assigned
}
zst = zsv_index_seek_row(uibuff->index, &opts, start_row);

Expand All @@ -121,6 +123,8 @@ static int read_data(struct zsvsheet_ui_buffer **uibufferp, // a new zsvsheet_
size_t find_len = zsvsheet_opts->find ? strlen(zsvsheet_opts->find) : 0;
size_t rows_searched = 0;
zsvsheet_screen_buffer_t buffer = uibuff ? uibuff->buffer : NULL;
if (uibuff && uibuff->has_row_num)
zsvsheet_opts->hide_row_nums = 1;

while (zsv_next_row(parser) == zsv_status_row &&
(rows_read == 0 || rows_read < zsvsheet_screen_buffer_rows(buffer))) { // for each row
Expand All @@ -140,6 +144,18 @@ static int read_data(struct zsvsheet_ui_buffer **uibufferp, // a new zsvsheet_
row_filter_len = row_filter ? strlen(row_filter) : 0;
}

// row number
size_t rownum_column_offset = 0;
if (rows_read == 0 && zsvsheet_opts->hide_row_nums == 0) {
// Check if we already have Row #
struct zsv_cell c = zsv_get_cell(parser, 0);
if (c.len == ZSVSHEET_ROWNUM_HEADER_LEN && !memcmp(c.str, ZSVSHEET_ROWNUM_HEADER, c.len)) {
zsvsheet_opts->hide_row_nums = 1;
if (uibuff)
uibuff->has_row_num = 1;
}
}

original_row_num++;
if (remaining_header_to_skip > 0) {
remaining_header_to_skip--;
Expand Down Expand Up @@ -169,12 +185,9 @@ static int read_data(struct zsvsheet_ui_buffer **uibufferp, // a new zsvsheet_
continue;
}

// row number
size_t rownum_column_offset = 0;
if (zsvsheet_opts->hide_row_nums == 0) {
if (rows_read == 0) // header
zsvsheet_screen_buffer_write_cell(buffer, 0, 0, (const unsigned char *)"Row #");
/////
zsvsheet_screen_buffer_write_cell(buffer, 0, 0, (const unsigned char *)ZSVSHEET_ROWNUM_HEADER);
else {
char buff[32];
int n = snprintf(buff, sizeof(buff), "%zu", original_row_num - 1);
Expand Down Expand Up @@ -210,9 +223,9 @@ static int read_data(struct zsvsheet_ui_buffer **uibufferp, // a new zsvsheet_
}

if (ui_status) {
pthread_mutex_lock(&uibuff->mutex);
if (uibuff->status)
free(uibuff->status);
pthread_mutex_lock(&uibuff->mutex);
uibuff->status = ui_status;
pthread_mutex_unlock(&uibuff->mutex);
}
Expand Down Expand Up @@ -240,17 +253,15 @@ static void *get_data_index(void *gdi) {
pthread_mutex_lock(mutexp);
*d->index_ready = 1;

if (d->uib && d->uib->status)
if (d->uib) {
free(d->uib->status);

if (d->row_filter != NULL) {
if (d->uib->index->row_count > 0) {
d->uib->dimensions.row_count = d->uib->index->row_count + 1;
asprintf(&d->uib->status, "(%" PRIu64 " filtered rows) ", d->uib->index->row_count);
} else
d->uib->status = NULL;
} else {
d->uib->status = NULL;
if (d->row_filter != NULL) {
if (d->uib->index->row_count > 0) {
d->uib->dimensions.row_count = d->uib->index->row_count + 1;
asprintf(&d->uib->status, "(%" PRIu64 " filtered rows) ", d->uib->index->row_count);
}
}
}
free(d);
pthread_mutex_unlock(mutexp);
Expand Down
3 changes: 3 additions & 0 deletions app/sheet/sheet_internal.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#ifndef ZSVSHEET_INTERNAL_H
#define ZSVSHEET_INTERNAL_H

#define ZSVSHEET_ROWNUM_HEADER "Row #"
#define ZSVSHEET_ROWNUM_HEADER_LEN strlen(ZSVSHEET_ROWNUM_HEADER)

enum zsvsheet_priv_status {
zsvsheet_priv_status_ok = 0,
zsvsheet_priv_status_memory,
Expand Down
8 changes: 7 additions & 1 deletion app/sheet/ui_buffer.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <unistd.h> // unlink()
#include <pthread.h>
#include <zsv/utils/index.h>
#include "index.h"

struct zsvsheet_ui_buffer {
char *filename;
Expand All @@ -13,6 +14,7 @@ struct zsvsheet_ui_buffer {
size_t cursor_col;
struct zsvsheet_input_dimensions dimensions;
struct zsv_index *index;
struct zsvsheet_index_opts *ixopts;
pthread_mutex_t mutex;

// input_offset: location within the input from which the buffer is read
Expand All @@ -35,15 +37,19 @@ struct zsvsheet_ui_buffer {
unsigned char index_ready;
unsigned char rownum_col_offset : 1;
unsigned char index_started : 1;
unsigned char _ : 6;
unsigned char has_row_num : 1;
unsigned char _ : 5;
};

void zsvsheet_ui_buffer_delete(struct zsvsheet_ui_buffer *ub) {
if (ub) {
if (ub->ext_on_close)
ub->ext_on_close(ub->ext_ctx);
zsvsheet_screen_buffer_delete(ub->buffer);
if (ub->ixopts)
ub->ixopts->uib = NULL;
free(ub->row_filter);
zsv_index_delete(ub->index);
free(ub->status);
if (ub->data_filename)
unlink(ub->data_filename);
Expand Down
14 changes: 13 additions & 1 deletion app/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ test-sheet: test-%: ${BUILD_DIR}/bin/zsv_%${EXE} worldcitiespop_mil.csv test-she
test-sheet-cleanup:
@rm -f tmux-*.log

test-sheet-all: test-sheet-1 test-sheet-2 test-sheet-3 test-sheet-4 test-sheet-5 test-sheet-6 test-sheet-7 test-sheet-8 test-sheet-9 test-sheet-10 test-sheet-11
test-sheet-all: test-sheet-1 test-sheet-2 test-sheet-3 test-sheet-4 test-sheet-5 test-sheet-6 test-sheet-7 test-sheet-8 test-sheet-9 test-sheet-10 test-sheet-11 test-sheet-12
@(for SESSION in $^; do ! tmux kill-session -t "$$SESSION" 2>/dev/null; done && ${TEST_PASS} || ${TEST_FAIL})

TMUX_TERM=xterm-256color
Expand Down Expand Up @@ -710,3 +710,15 @@ test-sheet-11: ${BUILD_DIR}/bin/zsv_sheet${EXE} worldcitiespop_mil.csv
tmux capture-pane -t $@ -p ${REDIRECT1} ${TMP_DIR}/$@.out && \
tmux send-keys -t $@ "q" && \
${CMP} ${TMP_DIR}/$@.out expected/$@.out && ${TEST_PASS} || (echo 'Incorrect output:' && cat ${TMP_DIR}/$@.out && ${TEST_FAIL}))

test-sheet-12: ${BUILD_DIR}/bin/zsv_sheet${EXE} worldcitiespop_mil.csv
@${TEST_INIT}
@echo 'set-option default-terminal "${TMUX_TERM}"' > ~/.tmux.conf
@(tmux new-session -x 80 -y 6 -d -s $@ "${PREFIX} $< worldcitiespop_mil.csv" && \
sleep 0.5 && \
tmux send-keys -t $@ "f" "el" "Enter" && \
sleep 0.5 && \
tmux send-keys -t $@ "f" "al" "Enter" && \
tmux capture-pane -t $@ -p ${REDIRECT1} ${TMP_DIR}/$@.out && \
tmux send-keys -t $@ "q" && \
${CMP} ${TMP_DIR}/$@.out expected/$@.out && ${TEST_PASS} || (echo 'Incorrect output:' && cat ${TMP_DIR}/$@.out && ${TEST_FAIL}))
10 changes: 5 additions & 5 deletions app/test/expected/test-sheet-10.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Row # Country City AccentCit Region Populatio Latitude Longitude
493036 gb rutupiae Rutupiæ G5 51.283333 1.333333
493037 lv briezucie Briezucie 03 56.95 27.583333
493038 gw quenem Quenem 04 12.228333 -15.64444
493039 gb ryde Ryde G2 24107 50.716667 -1.166667
(493039 filtered rows) 493039
999991 gb rutupiae Rutupiæ G5 51.283333 1.333333
999992 lv briezucie Briezucie 03 56.95 27.583333
999996 gw quenem Quenem 04 12.228333 -15.64444
999999 gb ryde Ryde G2 24107 50.716667 -1.166667
(493039 filtered rows) 999999
6 changes: 6 additions & 0 deletions app/test/expected/test-sheet-12.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Row # Country City AccentCit Region Populatio Latitude Longitude
117 pk basti faz Basti Faz 04 29.080758 70.984992
415 mx el salado El Salado 07 29.644444 -101.7666
590 af abbaskhel Abbaskhel 29 32.831993 69.12336
807 af `abdul `a `Abdul `A 10 31.229677 64.206403
(59456 filtered rows) 117
8 changes: 4 additions & 4 deletions app/test/expected/test-sheet-8.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Row # Country City AccentCity Region Population Latitude Longitude
493034 gb ruthven Ruthven V3 57.066667 -4.033333
493035 gb rutlandshire Rutlandshire L4 52.666667 -.666667
493036 gb rutupiae Rutupiæ G5 51.283333 1.333333
(493039 filtered rows) 493035
999987 gb ruthven Ruthven V3 57.066667 -4.033333
999990 gb rutlandshire Rutlandshire L4 52.666667 -.666667
999991 gb rutupiae Rutupiæ G5 51.283333 1.333333
(493039 filtered rows) 999990
7 changes: 7 additions & 0 deletions app/utils/index.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ struct zsv_index *zsv_index_new(void) {
return ix;
}

void zsv_index_delete(struct zsv_index *ix) {
if (ix) {
free(ix->array);
free(ix);
}
}

enum zsv_index_status zsv_index_add_row(struct zsv_index *ix, zsv_parser parser) {
struct zsv_index_array *arr = ix->array;
size_t len = arr->len, cap = arr->capacity;
Expand Down

0 comments on commit 5dfe914

Please sign in to comment.