test 3

[determin1st]

I think, it fixes:

https://github.com/gkz/LiveScript/issues/349

Because the compiler optimization, as well as the for loop, could give different result than while using .slice, my idea is to use .slice only in 2 easy to remember cases:

• variable[a til b]
• variable[a to b]

where a and b could be a literal, an expression or be empty. That way it could still use .slice, while not being a cognitive burden.

[determin1st]

Hey, @vendethiel does it take long for approval? Or how this works) and, I'll maybe move to another bug issue from this..

[rhendric]

Hey @determin1st, thanks for jumping in!

The first thing to get straight is whether or not this change is even desired. From my cursory review of #349, I'm not convinced that any proposed change is obviously right. I do think that the issue originally reported is something that merits fixing, so something should be done, but I don't have an opinion on what yet. Convince me you're right, or resume the conversation in #349 to get more of a consensus.

The second thing is, and I don't mean to discourage you, but this and #1086 are both terrible PRs for at least the following reasons:

• Their names and descriptions. A good PR should explain why I should bother to even look at it; otherwise, I won't.
• They include unrelated changes—both PRs include a commit that edits .gitignore, for example, which has nothing whatsoever to do with the main purpose of the PR. This PR seems to include commits from #1086, which I already rejected. I'm going to give you the benefit of the doubt and assume that this is because you are inexperienced with how Git and/or pull requests work, and not that you're trying to sneak something in and hoping I don't notice. But even so, if you want any contributions you make here to be accepted, you're going to have to do better.
• Work should be broken into commits that represent entire units of work, with clear and descriptive commit messages. (‘rev’ is, I shouldn't have to say, not a clear and descriptive commit message.) For fixing #349, unless something unusual comes up, one commit would be the correct number of commits.
• This PR touches a lot of characters that it doesn't need to. Changing indentation, reformatting otherwise unchanged lines, and renaming variables are all things that make the diff bigger and make it harder for me to review what you actually changed. A hard-to-review PR is an ignored PR. Please try to copy the existing style of the LS code as well as you can, and make the smallest possible changes to achieve the desired effect.
• Whenever possible, PRs that make observable changes to how code is compiled must include a test of the functionality being changed.

There may be other issues with the implementation here; I don't know, because I don't think it's worth my time to review this PR in this state. Get consensus on the desired effect, fix the above glaring issues, and then we can start talking about approval.

The above may sound harsh, but I'm not trying to push you away. Please believe me when I say that I would love for you to be making quality contributions to LiveScript's code; and once you start doing so, I will be happy to review and accept them. But there is either a learning or an effort gap that you need to cross in order to get there, and I won't compromise on that. If you're new to contributing to GitHub projects, you might want to look around on the web for resources in your native language about how to submit good pull requests. You should also be or get familiar enough with Git that you can rebase and amend commits to polish them up into an acceptable state.

[determin1st]

Well, thanks for explanation.

My concern, that Issues with "Bug" label are hard to traverse. Some of them have no decisions or no good examples to start from.. How to find some ready-for-implementation thing?

Changes in this PR may be reduced to 4 lines:

+1 line: https://github.com/gkz/LiveScript/pull/1088/files#diff-94df39e39af97137ba55977f88dd23a4R1259

and

+3 lines: https://github.com/gkz/LiveScript/pull/1088/files#diff-94df39e39af97137ba55977f88dd23a4R1323-R1326

with some comments inserted, i hope, it's clear what they do.

Other reformattings were done because.. hmm.. territorial marking thing. I feel that code becomes my own code and it helps me better understand the logic involved.. So, it can't be totally undone in a long run.

I'm okay with rewinding and throwing away first commit, no problem. Tests? Fully agree, is a must for such a project (they showed that my first attempt failed and required a revision).

As you won't compromize, lets pause/cancel this until either, I adapt or You trade. :handshake: