Skip to content

support for doubled file extensions ala issue #383#415

Merged
tannewt merged 3 commits into
adafruit:masterfrom
siddacious:doubled_file_extensions
Nov 8, 2017
Merged

support for doubled file extensions ala issue #383#415
tannewt merged 3 commits into
adafruit:masterfrom
siddacious:doubled_file_extensions

Conversation

@siddacious
Copy link
Copy Markdown

@siddacious siddacious commented Nov 7, 2017

Here's my fix. It's the first C I've written in quite a long time so by all means suggest any improvements that are warranted.

Copy link
Copy Markdown
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

A couple minor things.

Comment thread main.c Outdated
maybe_run("code.py", &result) ||
maybe_run("main.py", &result) ||
maybe_run("main.txt", &result);
const char *supported_filenames[] = {"code.txt", "code.py", "main.py", "main.txt",""};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about adding a macro to do this initialization? That way it can add the empty string.

STRING_LIST("code.txt", "code.py", "main.py", "main.txt")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't see a place for similar macros like supervisor/default/messages.h is for message strings. Should I put it in main.c or somewhere else?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In this file is fine. I'd probably just put it above maybe_run_list.

Comment thread main.c Outdated
return false;
bool maybe_run_list(const char ** filenames, pyexec_result_t* exec_result) {

for (int i = 0; *(filenames[i]); i++) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Implicit C int -> bool confuses my C++ and python brain. Mind making this filenames[i] != ""?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, that would be easier to read.

@siddacious
Copy link
Copy Markdown
Author

I made the changes and pushed.

Copy link
Copy Markdown
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for fixing this up!

@tannewt tannewt merged commit b4fc464 into adafruit:master Nov 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants