change the while with a for in parseMacroArgs

[coco875]

the goal of this pr is to fix when the argument is smaller then 1 (where -1 < 1 -1 equals false so he don't enter in the loop). I just change the while for a for loop.

[sauraen]

Good find!

[Dragorn421]

(I thought I reviewed this already, sorry)

[Dragorn421]

Changing to a for means the end += 1 below in the loop won't advance the loop as with the while

Overall this function should be basically unit tested, that would be a much easier way to tell correctness (doesn't need to be fancy, just a bunch of print(parseMacroArgs("...")) would do)

[coco875]

I just create a simple programme (not in the PR)

def new_parseMacroArgs(data: str):
    start = 0
    params: "list[str]" = []
    parenthesesCount = 0

    for end in range(len(data)):
        if data[end] == "(":
            parenthesesCount += 1
        elif data[end] == ")":
            parenthesesCount -= 1

        if (data[end] == "," or end == len(data) - 1) and parenthesesCount == 0:
            if end == len(data) - 1:
                end += 1
            param = "".join(data[start:end].split())
            params.append(param)
            start = end + 1

    return params

def old_parseMacroArgs(data: str):
    end = 0
    start = 0
    params: "list[str]" = []
    parenthesesCount = 0

    while end < len(data) - 1:
        end += 1
        if data[end] == "(":
            parenthesesCount += 1
        elif data[end] == ")":
            parenthesesCount -= 1

        if (data[end] == "," or end == len(data) - 1) and parenthesesCount == 0:
            if end == len(data) - 1:
                end += 1
            param = "".join(data[start:end].split())
            params.append(param)
            start = end + 1

    return params

def test_new_parseMacroArgs():
    assert new_parseMacroArgs("a, b, c") == ["a", "b", "c"]
    assert new_parseMacroArgs("abc") == ["abc"]
    assert new_parseMacroArgs("a") == ["a"]

def test_old_parseMacroArgs():
    assert old_parseMacroArgs("a, b, c") == ["a", "b", "c"]
    assert old_parseMacroArgs("abc") == ["abc"]
    assert old_parseMacroArgs("a") == [] # This is the bug

if __name__ == "__main__":
    test_new_parseMacroArgs()
    test_old_parseMacroArgs()

and the new version work like the old one except for the bug who are fix

[sauraen]

dragorn is right, you can't run the line end += 1 in a for loop iterating over end. I think the correct fix is to take the old code and change end = 0 at the beginning to end = -1.

[sauraen]

I apologize, I was wrong, the PR is correct. dragorn, look at the logic again. The only time end is modified is on the last iteration.