andmarti1424 / sc-im

sc-im - Spreadsheet Calculator Improvised -- An ncurses spreadsheet program for terminal
Other
4.8k stars 203 forks source link

Cannot set import_delimited_as_text configuration variable? #778

Closed Strahinja closed 1 year ago

Strahinja commented 1 year ago

My $HOME/.config/sc-im/scimrc contains the following:

set import_delimited_as_text=1

When starting sc-im, I'm getting the following error:

syntax error: set import_delimited_as_text=1

I have sc-im compiled from git source (commit b4cfc3fef326f78b8c683707f0819886ddcd82bd). My sc-im --version returns:

sc-im - version 0.8.4
-DNCURSES
-DMAXROWS 65536
-DUNDO
-DXLUA
-DDEFAULT_COPY_TO_CLIPBOARD_CMD="xclip -i -selection clipboard <"
-DDEFAULT_PASTE_FROM_CLIPBOARD_CMD="xclip -o -selection clipboard"
-DDEFAULT_OPEN_FILE_UNDER_CURSOR_CMD="scopen"
-DUSELOCALE
-DMOUSE
-DUSECOLORS
-D_XOPEN_SOURCE_EXTENDED
-D_GNU_SOURCE
-DSNAME="sc-im"
-DHELP_PATH="/usr/local/share/sc-im"
-DLIBDIR="/usr/local/share/doc/sc-im"
-DDFLT_PAGER="less"
-DDFLT_EDITOR="vim"
-DCONFIG_DIR=".config/sc-im"
-DCONFIG_FILE="scimrc"
-DHISTORY_DIR=".cache"
-DHISTORY_FILE="sc-iminfo"
-DINS_HISTORY_FILE="sc-iminfo"
-DHAVE_PTHREAD
-DAUTOBACKUP
syntax error: set import_delimited_as_text=1
Strahinja commented 1 year ago

I got this trace by setting yydebug = 1 and #define YYDEBUG 1, with the above one-line configuration file:

Starting parse
Entering state 0
Stack now 0
Reading a token
Next token is token S_SET ()
Shifting token S_SET ()
Entering state 85
Stack now 0 85
Reducing stack by rule 280 (line 1550):
-> $$ = nterm setlist ()
Entering state 213
Stack now 0 85 213
Reading a token
Next token is token WORD ()
Reducing stack by rule 108 (line 1098):
   $1 = token S_SET ()
   $2 = nterm setlist ()
-> $$ = nterm command ()
Entering state 96
Stack now 0 96
Next token is token WORD ()
Error: popping nterm command ()
Stack now 0
Shifting token error ()
Entering state 1
Stack now 0 1
Reducing stack by rule 132 (line 1242):
   $1 = token error ()
-> $$ = nterm command ()
Entering state 96
Stack now 0 96
Reading a token
Now at end of input.
Shifting token "end of file" ()
Entering state 223
Stack now 0 96 223
Stack now 0 96 223
Cleanup: popping token "end of file" ()
Cleanup: popping nterm command ()

Edit: Grepping for import_delimited_as_text in src/gram.y yields nothing. Has that configuration option been deprecated? If so, the documentation should be updated to reflect that.

Currently, the variable is queried in functions import_csv and import_markdown, but not defined in the configuration file grammar, making it unable to be parsed.

andmarti1424 commented 1 year ago

Thanks. I noticed the error. working on it.

andmarti1424 commented 1 year ago

Please try latest commit on dev branch. The configuration variable is now called "import_delimited_to_text".

Strahinja commented 1 year ago

Yes, that works. I tried importing a TSV file with the above configuration file, and it works as expected, numbers are imported as text.

Strahinja commented 1 year ago

It seems I spoke too soon. This issue (meaning parsing the variable from the configuration file) is solved, but I observed that the variable has effect only when using the :load command from within sc-im. When giving the TSV file as an argument on the command line:

sc-im myfile.tsv

numbers are imported as numbers (for example, 3 shows up in sc-im as 3.00 etc), even with the variable import_delimited_to_text=1. Of course, giving

sc-im --import_delimited_to_text myfile.tsv

works, but IMHO the configuration file should be respected even when explicit switches are not given on the command line.

andmarti1424 commented 1 year ago

try setting the configuration variable correctly in your scimrc. its name was modified.

Strahinja commented 1 year ago

I have edited the configuration file so it reads

set import_delimited_to_text=1

Within sc-im, :set gives

autobackup=0
autocalc=1
autowrap=0
command_timeout=3000
copy_to_clipboard_delimited_tab=0
debug=0
default_copy_to_clipboard_cmd=xclip -i -selection clipboard <
default_open_file_under_cursor_cmd=scopen
default_paste_from_clipboard_cmd=xclip -o -selection clipboard
exec_lua=1
external_functions=0
half_page_scroll=1
help=0
ignore_hidden=0
ignorecase=0
import_delimited_to_text=1
input_bar_bottom=0
mapping_timeout=1500
newline_action=0
nocurses=0
numeric=0
numeric_decimal=1
numeric_zero=1
overlap=0
quiet=0
quit_afterload=0
show_cursor=0
tm_gmtoff=3600
trigger=1
truncate=0
underline_grid=0
xlsx_readformulas=0

Press 'q' and then ENTER to return.

Still, as stated, :load myfile.tsv works (any number is displayed as text in sc-im), but executing

sc-im myfile.tsv

yields decimal representation of numbers, ie they aren't displayed as text.

Strahinja commented 1 year ago

The same in screenshots: sc-im-001

Result of sc-im test.tsv: sc-im-002

Result of :load test.tsv: sc-im-003

andmarti1424 commented 1 year ago

yep. this is a different problem. i cannot remember right now why but taking a look at main.c:282 image load_rc is after load_file..

Strahinja commented 1 year ago

Is there any update on this? Would it be possible to put the call to load_rc before load_file?

andmarti1424 commented 1 year ago

Not really, but you can try swapping those lines and do the honor of testing it.

Strahinja commented 1 year ago

I made the swap and also removed some now redundant checks. The code in the PR works with regards to this issue.

andmarti1424 commented 1 year ago

@Strahinja Thank you. the thing is the following: i have some memory flaws. its a shame i didnt comment the code on that. but i know that order was like that for a reason. I cannot remember right now. feel free to test it for some time. well then see if we can merge it.

Strahinja commented 1 year ago

Ok, I'll see if there are any leaks and other errors using ASAN and valgrind in the coming days and try to organize startup a bit better. I'll update this PR when I make some progress.

Strahinja commented 1 year ago

I tried compiling sc-im with

CFLAGS='-fsanitize=address' LDFLAGS='-fsanitize=address' make

and running the resulting program did produce some errors. Attached is the output from sc-im built from my branch (asan-myfork.txt) and the one built from your current dev branch (asan-original-dev.txt). They are very similar, so I'll quote the most interesting parts:

ERROR: AddressSanitizer: global-buffer-overflow on address 0x557aa3ebfc78 at pc 0x557aa3e23dde bp 0x7ffdb88bc2f0 sp 0x7ffdb88bc2e0
READ of size 1 at 0x557aa3ebfc78 thread T0
    #0 0x557aa3e23ddd in yylex /home/strajder/src/sc-im/src/lex.c:162
    #1 0x557aa3e9ca05 in yyparse /home/strajder/src/sc-im/src/y.tab.c:2979
    #2 0x557aa3def877 in readfile /home/strajder/src/sc-im/src/file.c:896
    #3 0x557aa3de8beb in load_rc /home/strajder/src/sc-im/src/file.c:156
    #4 0x557aa3e286e6 in main /home/strajder/src/sc-im/src/main.c:277
    #5 0x7f43fbe6b2cf  (/usr/lib/libc.so.6+0x232cf)
    #6 0x7f43fbe6b389 in __libc_start_main (/usr/lib/libc.so.6+0x23389)
    #7 0x557aa3ddff84 in _start ../sysdeps/x86_64/start.S:115

and

SUMMARY: AddressSanitizer: global-buffer-overflow /home/strajder/src/sc-im/src/lex.c:162 in yylex

The error is in lex.c, line 162. This is the relevant part of the code: https://github.com/andmarti1424/sc-im/blob/2b3b91d1f615af0a6c8b47e41383a6d3af87060d/src/lex.c#L162

The most important part is that reordering of reading the configuration and the file itself didn't change the situation with the buffer overflow to better nor worse, so I think there is no reason it should not be implemented.

andmarti1424 commented 1 year ago

merged PR 783 to solve this!