japsmr/swagshell

Fork 1

Code Issues 1 Pull requests Projects Releases Packages Wiki Activity

feat: tokenizer #6

Merged

poz merged 4 commits from :tokenizer into main

2023年08月30日 14:17:46 +02:00

Conversation 2 Commits 4 Files changed 5 +419 -55

redzimski commented

2023年08月29日 20:42:34 +02:00

Contributor

Copy link

1 commit xd

redzimski added 1 commit

2023年08月29日 20:42:35 +02:00

feat: tokenizer 40bcadca69

redzimski added 1 commit

2023年08月29日 20:42:50 +02:00

Merge branch 'main' into tokenizer 157deb6641

poz requested changes

2023年08月29日 22:16:36 +02:00

src/cmdparse.c Outdated

@ -50,0 +17,4 @@

 TOKEN_ASSIGNMENT_WORD, // assigned during parsing

 TOKEN_NAME, //

 TOKEN_IO_NUMBER, //

 TOKEN_KEYWORD //

poz commented

2023年08月29日 21:16:35 +02:00

Owner

Copy link

either remove all of these comments or actually explain TOKEN_ASSIGNMENT_WORD and TOKEN_IO_NUMBER (I'm for removing)

either remove all of these comments or actually explain `TOKEN_ASSIGNMENT_WORD` and `TOKEN_IO_NUMBER` (I'm for removing)

redzimski marked this conversation as resolved

src/cmdparse.c Outdated

@ -62,0 +34,4 @@

#define SINGLE_QUOTE '\''

#define NEWLINE '\n'

#define DOLLAR '$'

#define BACKQUOTE '`'

poz commented

2023年08月29日 22:04:36 +02:00

Owner

Copy link

I've always seen this called a backtick but if it's backquote in the docs then let's keep that

redzimski marked this conversation as resolved

src/cmdparse.c

@ -66,0 +45,4 @@

do { \

if(!is_end()) { \

tk_state.prev_char = c; \

c = *(++tk_state.source); \

poz commented

2023年08月29日 21:24:59 +02:00

Owner

Copy link

this macro assumes that there's a c, provide it as an argument

same with the macro below

this macro assumes that there's a `c`, provide it as an argument same with the macro below

redzimski marked this conversation as resolved

src/cmdparse.c

@ -66,0 +47,4 @@

tk_state.prev_char = c; \

c = *(++tk_state.source); \

} \

} while(0)

poz commented

2023年08月29日 21:18:23 +02:00

Owner

Copy link

this might be thinking about optimizations too early but do the compilers optimize these away?

we could check in some disassembler I think?

this might be thinking about optimizations too early but do the compilers optimize these away? we could check in some disassembler I think?

redzimski marked this conversation as resolved

src/cmdparse.c

@ -66,2 +50,3 @@

} while(0)

NOT_IMPLEMENTED;

#define TK_ADD_TO_VALUE(c) \

poz commented

2023年08月29日 21:55:38 +02:00

Owner

Copy link

both of these can be functions, please do that

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +61,4 @@

static struct {

char *source;

poz commented

2023年08月29日 21:21:25 +02:00

Owner

Copy link

I'm guessing source means the current string, we're parsing, maybe rename to something like input or input_string? it doesn't make much sense when you're thinking about interactive mode

I'm guessing `source` means the current string, we're parsing, maybe rename to something like `input` or `input_string`? it doesn't make much sense when you're thinking about interactive mode

redzimski marked this conversation as resolved

src/cmdparse.c Outdated

@ -68,0 +66,4 @@

token_type_t next_type;

} tk_state = {0};

poz commented

2023年08月29日 21:19:29 +02:00

Owner

Copy link

minor thing but please remove one of the empty lines here and above on 61-62

redzimski marked this conversation as resolved

src/cmdparse.c Outdated

@ -68,0 +72,4 @@

}

static bool is_operator_first_char(char c) {

for(size_t i = 0; i < sizeof(operators)/sizeof(operators[0]); i++)

poz commented

2023年08月29日 21:26:00 +02:00

Owner

Copy link

sizeof(arr)/sizeof(arr[0]) is probably gonna be used a lot over the place, maybe make it a function / macro in util.h?

`sizeof(arr)/sizeof(arr[0])` is probably gonna be used a lot over the place, maybe make it a function / macro in `util.h`?

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +79,4 @@

}

static bool can_operator_be_continued(char *cur_op, char c) {

size_t i, j, sc = strlen(cur_op);

poz commented

2023年08月29日 21:31:40 +02:00

Owner

Copy link

please put i and j declarations inside the loops, also maybe rename sc to cur_op_len?

please put `i` and `j` declarations inside the loops, also maybe rename `sc` to `cur_op_len`?

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +81,4 @@

static bool can_operator_be_continued(char *cur_op, char c) {

size_t i, j, sc = strlen(cur_op);

char *newop;

malloc_safe(newop, sc + 1 /* c */ + 1 /* nul */);

poz commented

2023年08月29日 21:29:22 +02:00

Owner

Copy link

small thing but these at first glance look like you did sc + 1 + c + 1 + nul earlier but uncommented these for debugging or something and forgot to remove them

maybe put these in a comment a line above lined up vertically with the ones?

small thing but these at first glance look like you did `sc + 1 + c + 1 + nul` earlier but uncommented these for debugging or something and forgot to remove them maybe put these in a comment a line above lined up vertically with the ones?

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +85,4 @@

for(i = 0; i < sizeof(operators)/sizeof(operators[0]); i++) {

const char *op = operators[i];

size_t so = strlen(op);

poz commented

2023年08月29日 21:32:04 +02:00

Owner

Copy link

same here, op_len instead of so

same here, `op_len` instead of `so`

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +98,4 @@

for(j = 0; j < so && j < sc; j++)

if(newop[j] != op[j])

 goto skip_op; // move onto the next operator

poz commented

2023年08月29日 21:30:09 +02:00

Owner

Copy link

why not just continue here?????

poz commented

2023年08月29日 22:23:01 +02:00

Owner

Copy link

nvm I'm blind and didn't see that you wanna continue the outer loop

poz marked this conversation as resolved

src/cmdparse.c Outdated

@ -68,0 +121,4 @@

static token_t _next_token(void) {

token_t t = {0};

char quote_char = '0円', c;

bool quoted = false, request_more_input = false;

poz commented

2023年08月29日 21:34:20 +02:00

Owner

Copy link

maybe split the declarations into one per line?

redzimski commented

2023年08月30日 11:52:44 +02:00

Author

Contributor

Copy link

poz commented

2023年08月30日 12:15:15 +02:00

Owner

Copy link

why not

redzimski commented

2023年08月30日 13:39:56 +02:00

Author

Contributor

Copy link

i am saving disk space (few less bytes)

poz commented

2023年08月30日 13:58:15 +02:00

Owner

Copy link

at least swap them, I missed the c the first time

poz marked this conversation as resolved

src/cmdparse.c

@ -68,0 +189,4 @@

continue;

}

if(c == DOLLAR && !quoted) {

poz commented

2023年08月29日 21:57:58 +02:00

Owner

Copy link

envvars can also be inside double quotes

redzimski marked this conversation as resolved

src/cmdparse.c Outdated

@ -68,0 +257,4 @@

continue;

}

if(c == HASH) {

poz commented

2023年08月29日 22:07:50 +02:00

Owner

Copy link

maybe we should parse the comment as a token too? IDK if it'll be of any use though

redzimski commented

2023年08月30日 13:38:21 +02:00

Author

Contributor

Copy link

no
it wont

no it wont

redzimski marked this conversation as resolved

src/cmdparse.c

@ -68,0 +271,4 @@

TK_STATE_ADVANCE();

}

if(t.type == TOKEN_UNKNOWN && (t.value == NULL || !t.value[0]) && is_end())

poz commented

2023年08月29日 22:08:49 +02:00

Owner

Copy link

maybe t.value[0] == '0円'?

maybe `t.value[0] == '0円'`?

redzimski marked this conversation as resolved

src/cmdparse.c Outdated

@ -68,0 +306,4 @@

============================================================ */

static void remove_escaped_newlines(char **command) {

size_t len = strlen(*command), i;

poz commented

2023年08月29日 22:12:25 +02:00

Owner

Copy link

put the i back into the loop where it belongs

put the `i` back into the loop where it belongs

redzimski marked this conversation as resolved

src/util.h Outdated

@ -5,3 +7,4 @@

#define NOT_IMPLEMENTED assert("Not implemented" && 0)

#define malloc_safe(p, sz) \

poz commented

2023年08月29日 22:16:26 +02:00

Owner

Copy link

make these 2 functions as well

redzimski marked this conversation as resolved

redzimski added 1 commit

2023年08月30日 13:47:14 +02:00

fix: fixes and tweaks to tokenizer e09acdd34b

poz requested changes

2023年08月30日 14:03:47 +02:00

src/command.c

@ -6,2 +6,4 @@

command_t cmd;

 // ignore empty command (NULL or "")

if(!command || !command[0])

poz commented

2023年08月30日 14:03:46 +02:00

Owner

Copy link

will this catch blank commands? (all whitespaces)

redzimski commented

2023年08月30日 14:15:58 +02:00

Author

Contributor

Copy link

no but those dont segfault

poz commented

2023年08月30日 14:17:34 +02:00

Owner

Copy link

ok let's leave it for now, in the future maybe we can trim the whole string before doing anything on it

poz marked this conversation as resolved

redzimski added 1 commit

2023年08月30日 14:14:12 +02:00

chore: move decl ca9e7c3d19

poz merged commit e91421bfce into main

2023年08月30日 14:17:46 +02:00

poz referenced this pull request from a commit

2023年08月30日 14:17:47 +02:00

Merge pull request 'feat: tokenizer' (#6) from krizej/swagshell:tokenizer into main

Reviewers

No reviewers

poz

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

japsmr/swagshell!6

No description provided.

Delete branch ":tokenizer"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?