Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Style of Syntax error #333

Closed
wants to merge 3 commits into from
Closed

Better Style of Syntax error #333

wants to merge 3 commits into from

Conversation

JakobDev
Copy link
Contributor

I think the new looks more like native LUA.

2017-06-23_16 31 28

@SquidDev
Copy link
Contributor

SquidDev commented Jun 23, 2017

You don't need the -1 on the sub calls - Lua will default to the end of the string.

Worth noting that this is also fixed by #163 (/me does puppy eyes at @dan200).


Edit: From my testing, return false, err:sub(14) is sufficient to strip the bios.lua element off. Just wondering why you went for the current method rather than this?

@SquidDev
Copy link
Contributor

SquidDev commented Jun 24, 2017

It's also worth noting that name can be nil, which will result in load erroring. I think a simple err:sub(14) is the best solution here.

@JakobDev
Copy link
Contributor Author

JakobDev commented Jun 24, 2017

@SquidDev Have you any example who name is nil, so I can look how to fix this?

@SquidDev
Copy link
Contributor

@Wilma456 You're allowed to use load - something like load("return 1") is legal. Trying to do load("1") crashes with bios.lua:22: attempt to get length of nil.

@JakobDev
Copy link
Contributor Author

@SquidDev Should be fixed now

@SquidDev
Copy link
Contributor

SquidDev commented Jun 26, 2017

@Wilma456 Trying load("x") produces nilg"]: '=' expected, which is far from ideal :).

From my testing, return false, err:sub(14) is sufficient to strip the bios.lua element off. Just wondering why you went for the current method rather than this?

@JakobDev
Copy link
Contributor Author

if err == nil then return should protect from this. Crazy.

@Wojbie
Copy link
Contributor

Wojbie commented Jun 26, 2017

Why did you put read char limit #330 into this PR?

@JakobDev
Copy link
Contributor Author

@Wojbie Sorry, this was in the same file and I had not see this. It is now removed.

@Wojbie
Copy link
Contributor

Wojbie commented Jun 26, 2017

You forgot to push.

@JakobDev
Copy link
Contributor Author

No. In my Repository it is fixed. https://github.com/Wilma456/ComputerCraft-1/blob/syntaxerr/src/main/resources/assets/computercraft/lua/bios.lua But is not shown here. Maybe it's a Bug of Github. I wait a few hours.

@SquidDev
Copy link
Contributor

if err == nil then return should protect from this. Crazy.

That won't do anything: load should never return a nil error message so that check is redundant. The issue is that your :sub code is invalid.

From my testing, return false, err:sub(14) is sufficient to strip the bios.lua element off. Just wondering why you went for the current method rather than this?

@dan200
Copy link
Owner

dan200 commented Jun 28, 2017

This is very hacky

@dan200 dan200 closed this Jun 28, 2017
@JakobDev JakobDev deleted the syntaxerr branch June 29, 2017 16:02
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.

4 participants