Added functionality to run hedy files locally

[sjensen19]

Added functionality to run your hedy files locally. You can install the dependencies using pip install -r requirements.txt.
Then use python3 hedy.py LEVEL PATH_TO_FILE to launch your file

[sjensen19]

Now changed to defining level in code file (python3 hedy.py PATH_TO_FILE)
Add the line #LEVEL LEVEL to the top of the code file

[fpereiro]

Hi @Sven-Developer. Thank you for your contribution! We'll be reviewing the PR shortly :).

[fpereiro]

@Sven-Developer I tried it locally and it works well! Thanks for this contribution!

Could you add a small paragraph here (https://github.com/Felienne/hedy/blob/master/CONTRIBUTING.md#run-hedy-code-on-your-machine) in your PR to describe how to use this functionality? It'd be similar to what you specified in the PR (pass the path to the hedy file as an argument to the command, and specify the level at the top of the file with a line of the form #level N).

Am not sure why the tests are not passing. Will discuss with @Felienne how to figure this out and as soon as that's cleared out we can proceed to merging.

Will keep you updated!

[sjensen19]

I dont know either why the test are not passing. I will make a description how to use it.

[sjensen19]

I have updated the contributing file with the new feature

[fpereiro]

Thank you for the docs! I'll see if I can fix the test issue - I have the same error now locally so I should be able to get to the bottom of it.

[sjensen19]

So I thought i saw the problem, because i edited the excecute function with a extra argument, I thought it ran into a problem there (for example other files that use that same function will fail the excecution because there is a extra argument.). So I copied the function and made it a seperate one so it would not complain about that. Still not fixed the error...

[fpereiro]

@Sven-Developer I tested again and it's not the naming. The strange thing is that execute function doesn't seem to be used anywhere else, but still changing it triggers an error. I tried just copying your changes onto the latest master (since I suspected that perhaps you were missing some code from the latest commits for the tests to pass) but the issue still remains.

As soon as I clear up some urgent UI fixes, I'll be back to this to see how we can make the tests pass. I'll keep you updated.

[rix0rrr]

@fpereiro @Sven-Developer, ik heb uitgevonden waarom de code het niet doet.

Het komt omdat je in hedy.py op het hoogste niveau in de file code bent gaan schrijven:

if len(sys.argv) > 1:
     LEVEL = 1
     FILE = sys.argv[1]
     ...enzovoort...

Die code wordt uitgevoerd als je hedy.py rechtstreeks uitvoert op de command-line, dus als je:

$ python hedy.py BESTAND.hedy

Intypt. Maar diezelfde code wordt óók uitgevoerd als iemand anders een ander bestand uitvoert, waarin

import hedy

staat. Bijvoorbeeld in app.py:

https://github.com/Felienne/hedy/blob/b4359bf202c952fd1d946e5a91a57abcbe2991b7/app.py#L5

En ook in tests.py:

https://github.com/Felienne/hedy/blob/b4359bf202c952fd1d946e5a91a57abcbe2991b7/tests.py#L2

Dus als je gewoon code typt in de linkerkantlijn, en iemand doet dan:

$ python3 -m unittest tests.py

Dan wordt tests.py geladen, en vervolgens hedy.py geladen, en runt jouw code! Maar let op wat er nu gebeurt!

Jouw if kijkt naar sys.argv, maar daar staat nu iets heel anders in dan je denkt!

In dit geval zit daar het volgende in:

['python3 -m unittest', 'tests.py']

En wat gebeurt er nu? Jouw stukje code denkt dat sys.argv[1] (waar "tests.py" in zit) een Hedy programma is dat uitgevoerd moet worden.

Dus gaat die nu vrolijk tests.py proberen te parsen als een Hedy programma, en dat gaat natuurlijk mis. Dan komt er een exception, en stopt het programma!

---

Lange uitleg, maar hoe fixen we dat nu?

Het problem is dat hedy.py eigen een dubbele dienst draait. Je wil dat het zowel werkt als je het rechtstreeks uitvoert (python3 hedy.py BESTAND.hedy) en als library ingeladen kan worden (import hedy).

Dus je wil eigenlijk dat jouw code ALLEEN uitgevoerd wordt in het eerste geval, en niet in het tweede.

Dat doe je door het volgende voor je code te zetten:

if __name__ == '__main__':
    # hier komt de code die uitgevoerd moet worden als Hedy als los programma draait
    ...

Zoals je ziet is dit nogal foutgevoelig, en heel verwarrend als het misgaat. Het kán dus wel, maar is misschien niet zo handig om te doen.

Ik zou je aanraden om een nieuw script te maken om lokaal Hedy programma's uit te voeren. Misschien noem je dat bijvoorbeeld runhedy.py, en die wordt ALLEEN uitgevoerd met python3 runhedy.py BESTAND.

En dan laat je hedy.py lekker zó dat het alleen de bedoeling is om geimporteerd te worden met import hedy.

[sjensen19]

Ah kijk, dat verklaart een hele hoop, ik zal morgen een nieuwe commit doen waarin ik het zal oplossen. Dank voor het meedenken

[rix0rrr]

@Sven-Developer ik heb ook even de vrijheid genomen om je code een beetje te herschrijven. Ik hoop dat je dat goed vindt. Ik zal je meteen uitleggen waarom ik dat gedaan heb:

# 1️⃣
def main(): 
    # 2️⃣ argv0 is the name of the script, so let's cut that off to
    # make the indices easier to reason about.
    args = sys.argv[1:]

    # 3️⃣ Let's be nice to the user and explain how this program is supposed
    # to be used if they're doing it wrong.
    if len(args) == 0 or args == ['-h'] or args == ['--help']:
        sys.stderr.write('Run a Hedy source file.\n')
        sys.stderr.write('\n')
        sys.stderr.write('Usage: hedy.py FILE\n')
        sys.stderr.write('\n')
        sys.stderr.write('Select a level by putting "#level <number>" on the first line by itself.\n')
        # 8️⃣
        sys.exit(1)

    # 4️⃣
    level = 1
    filename = args[0]

    #  5️⃣ Use the 'with' block to close the file as soon as we don't need
    # it anymore.
    with open(filename, 'r') as f:
        lines = f.readlines()

    # Look for '#level <number>' on the first line
    if lines[0].lower().startswith('#level'):
        parts = lines[0].split(' ')
        if len(parts) >= 2:
            level = int(parts[1])

    # 6️⃣ Keep only all lines that do NOT start with '#' (those are comments),
    # then join them back to a big string.
    program = '\n'.join([line
        for line in lines
        if not line.startswith('#')])

    try:
        execute_locally(program, level)
    except HedyException as e:
        # 7️⃣ If we have a Hedy parsing exception, print it slightly nicer than
        # before.
        sys.stderr.write('An error occurred: %s\n' % e.error_code)
        sys.stderr.write('Error attributes:\n')
        for key, value in e.arguments.items():
            sys.stderr.write(f'{key}={value}\n')
        # 8️⃣
        sys.exit(1)

if __name__ == '__main__':
    main()

1️⃣ Ja, je kan je code meteen in het if __name__ == '__main__': blok zetten, maar functies zijn wel zo netjes. Een main() functie waar de hoofdlogica van je programma in staat is heel traditioneel en duidelijk voor iedereen. Bovendien kun je de code beter organiseren, van belangrijke functies bovenaan naar minder belangrijke functies verderop in de file. In main() mag je functies aanroepen die ONDER de def main() gedefiniëerd staan, en dat had niet gemogen als je de code "rechtstreeks" in de if typt. Ik kan wel meer uitleggen waarom dat is (heeft te maken met hoe Python werkt), maar voor nu mag je het van me aannemen :).

2️⃣ Omdat ik sys.argv (persoonlijk) verwarrend vind, sloop ik er altijd het eerste element vanaf. Wat je overhoudt zijn de ECHTE argumenten. Wel zo duidelijk!

3️⃣ Het is wel zo handig dat als je vergeten bent hoe je een programma moet gebruiken, het programma het je vertelt! Laten we aardig zijn voor de gebruiker (dat ben je zelf ook over een maand!)

4️⃣ Ik zag dat al jouw variabelen met HOOFDLETTERS waren. Het is gebruikelijker om ze met kleine letters te schrijven. HOOFDLETTERS worden voornamelijk gebruikt voor constanten.

5️⃣ Een programma mag maximaal een bepaald aantal bestanden tegelijk open hebben. Het is dus verstandig om ze te sluiten als je ze niet meer nodig hebt, zodat je programma niet opeens stopt met een fout omdat je over de grens heen gaat! (Ja, voor dit programma maakt het niet uit want het is er maar ééntje, en Python helpt je ook nog eens met automatisch sluiten zodat het niet per sé nodig is. Doe het toch maar, want het is een goede gewoonte om aan te wennen. Van die gewoonte ga je later nog veel lol hebben).

6️⃣ Ik heb je lus omgeschreven naar een "list comprehension" met een "join". De list comprehension: [line for line in lines if not line.startswith('#')] houdt alle regels over die NIET beginnen met een #. Vervolgens plak je ze allemaal aan elkaar tot een string, met \n tussen de regels. Om redenen (die ik ook wel kan uitleggen als je wil) is dit sneller dan een herhaalde += op een string, en dus ook een goede gewoonte. Bovendien vind ik (persoonlijk) list comprehensions leuker/mooier dan een for lus, omdat je ook geen tijdelijke variabelen nodig hebt.

7️⃣ Het viel me op dat HedyExceptions nogal lelijk geprint werden, en niet zo behulpzaam waren. We vangen hem dus af en printen de inhoud ervan op een duidelijkere manier.

8️⃣ Als er iets misgaat stoppen we het programma altijd met een foutcode. Dat is handig als mensen eromheen willen programmeren in een script (bijvoorbeeld in Batch of bash), zodat ze kunnen controleren of alles goed gegaan is. En foutuitvoer gaat altijd naar stderr in plaats van stdout.

[sjensen19]

Ah top, ik zal er naar kijken en het morgen proberen het zo goed mogelijk te implementeren in de code!

[rix0rrr]

Je kan de code van hierboven copy/pasten 😉

[Felienne]

Je kan de code van hierboven copy/pasten 😉

Maar je leert natuurlijk meer als je het zelf probeert :)

[sjensen19]

@rix0rrr Hoezo gebruik je sys.stderr.write in plaats van print?

[rix0rrr]

@rix0rrr Hoezo gebruik je sys.stderr.write in plaats van print?

Om te beginnen kleine grammaticacorrectie: je bedoelt "waarom" in plaats van "hoezo" 😛.

Tekstprogramma's hebben twee stromen van uitvoer: die heten "standard output" (stdout) en "standard error" (stderr). De bedoeling is dat je normale uitvoer op stdout print, en fouten op stderr. Dat wordt vooral relevant als je op UNIX/Linux werkt, waar het gebruikelijk is om de uitvoer van het éne programma als invoer van het volgende programma te gebruiken.

Google er maar eens naar. Hier is bijvoorbeeld een hit die er wel nuttig uit ziet: https://retrocomputing.stackexchange.com/questions/11499/what-was-the-point-of-separating-stdout-and-stderr

In Python schrijf je naar stdout en stderr op deze manier:

sys.stdout.write('hallo\n')
sys.stderr.write('doei\n')

Omdat je printen heel vaak doet en je uitvoer MEESTAL naar stdout moet (want wie besteedt er nou de meeste tijd aan het printen van fouten?) is print() een kortere vorm om te printen naar stdout. Je zou print() zelf kunnen maken, en het ziet er dan ongeveer uit als:

def print(uitvoer):
    sys.stdout.write(uitvoer + '\n')

[sjensen19]

Ik heb de code zelf een beetje anders gemaakt zodat je ook het level als argument kan meegeven.

Nu is het dus geworden:
python3 hedy.py <filename> met als optioneel argument --level <level>

[Felienne]

Goed werk Sven! De tests passen nu netjes.

Het is wel iets veiliger en overzichtelijker als we jouw logica voor het offline runnen in een andere file zetten, zoals @rix0rrr hier helemaal boven voorstelt:

Ik zou je aanraden om een nieuw script te maken om lokaal Hedy programma's uit te voeren. Misschien noem je dat bijvoorbeeld runhedy.py, en die wordt ALLEEN uitgevoerd met python3 runhedy.py BESTAND.

En dan laat je hedy.py lekker zó dat het alleen de bedoeling is om geimporteerd te worden met import hedy.

Lukt het jou om dat te maken?

[sjensen19]

Zeker, ik zal dat nu gaan doen

[sjensen19]

Het is gelukt 😄

[sjensen19]

Was vergeten de nieuwe versie van de normale hedy.py te pushen, nu alsnog gedaan

[sjensen19]

Ik zal trouwens nu ook even de contributing.md file aanpassen dat alles daar ook klopt 😄

[sjensen19]

Nu zou als het goed is alles gedaan moeten zijn wat nodig is.

[Felienne]

Gemerged! Goed werk Sven!!!

[fpereiro]

@rix0rrr Thank you for the amazingly detailed feedback and for getting to the root of the issue!!

@Sven-Developer Thanks again for your contribution!