KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
691 stars 229 forks source link

Allowing dots in barewords caused weird behavior on varible names.a #1396

Open Dunbaratu opened 8 years ago

Dunbaratu commented 8 years ago

In order to allow things like this to work:

copy myfile.ks from 0.

we had to change the rules about bareword identifiers so they allow dots, so that myfile.ks gets seen as a single token.

But I just noticed a weird annoying effect this has on attempting to use dots in identifiers in other contexts.

Let's say you forget that kerboscript uses colons for members and you type this:

kuniverse.debuglog("===message===").

instead of the correct:

kuniverse:debuglog("===message===").

The error the parser gives you will be quite confusing. It will claim there is no such identifier as kuniverse.debuglog. It takes the whole thing as if it was one identifier.

That made me think a bit more and I tried this:

set x.y.z to 5.
print x.y.z.
x.y.z

It almost allowed x.y.z to be the actual name of a variable. It didn't issue any complaint when I said set x.y.z to 5. It just silently failed to work, and when I tried using x.y.z like it was a variable name, it was back to being interpreted like it had never been assigned and was thus a bareword filename.

hoylemd commented 8 years ago

Why append the '.ks' extension to the filename* in the first place? All script files will be .ks, so the extension is vestigial at the in-game terminal, right?

*In the in-game terminal I mean, not the actual filename. ES6 (Javascript) does this with includes - leaving off the extension because it's implied.

hvacengi commented 8 years ago

@hoylemd because we have two valid forms of executable scripts, and we can't outright make that decision for the user. If there are files "lib_orbit.ks" and "lib_orbit.ksm" there needs to be a way to specify one or the other, even if we default to one of those extensions if none is given. Also, as things are written now, scripts with any extension are valid (i.e. .txt, .script, .ihateks).

hoylemd commented 8 years ago

Hmm, what's the difference between .ks and .ksm? In cases of no ambiguity, it should be fine to ignore extensions, and establishing an explicit priority of file extensions could go a long way.

My point is that allowing periods in bare words might be a lot more trouble than it's worth, especially if it's just to accomodate file name collisions. On Tue, Feb 2, 2016 at 8:12 AM Brad White notifications@github.com wrote:

@hoylemd https://github.com/hoylemd because we have two valid forms of executable scripts, and we can't outright make that decision for the user. If there are files "lib_orbit.ks" and "lib_orbit.ksm" there needs to be a way to specify one or the other, even if we default to one of those extensions if none is given. Also, as things are written now, scripts with any extension are valid (i.e. .txt, .script, .ihateks).

— Reply to this email directly or view it on GitHub https://github.com/KSP-KOS/KOS/issues/1396#issuecomment-178570163.

Dunbaratu commented 8 years ago

Allowing periods in bare words has been present for a long time now in kOS. When you say "might be a lot more trouble than it's worth", you're not talking about removing a proposed new feature, but about breaking backward compatibility with an existing old feature.

This problem has been here a while and I only just noticed it yesterday, meaning it may be not that severe if nobody complained about it before. Really the only problem is the lack of a clear error message to the user, and that can be added.

hoylemd commented 8 years ago

You're right - breaking backwards compatability is generally something to be avoided, but I don't think it's something to be avoided at all costs. If it makes the language/mod better, it's worth consideration.

I personally don't use the file extensions in my copy or run lines (I actually wasn't even aware that I could), so it seems natural to me to not use them. That's just my experience though. If using the extensions is a common practice, then leaving them in bare words is definitely the right move.

In any case, a clearer error message is good to have now. :+1: