fix: handle ctypes.windll attribute error on Linux#5
Conversation
…ctionaries, and enhance comment handling in Widgets
|
Danke für den Fix! Ich habe das Projekt schon lange nicht mehr angefasst, aber es ist schön zu sehen, dass noch Interesse an Verbesserungen besteht. Ich werde die Tage, wenn ich Zeit finde, mir das mal anschauen und sicherlich nach Anpassungen mergen. :) Ich sehe in den Commits, dass du noch mehr als nur den Bug behoben hast. Magst du jede Änderung (Fonts, Error Handling, neue Befehle, Code Highlighting, Auto-Edits, etc.) in dem PR auflisten und eine kurze Beschreibung und Begründung dafür hinzufügen? Das macht das Nachvollziehen einfacher und ist gut für die Vollständigkeit. |
Blyfh
left a comment
There was a problem hiding this comment.
Erstmal: Vielen Dank für die viele Arbeit! Ich weiß nicht, ob ich das in näherer Zeit alles integrieren kann, da ich sehr beschäftigt bin. Ich gebe mein bestes.
Zu den Fonts: Finde ich sehr gut. Cross-Platform war immer das Ziel. In profile.dict ist aber noch eine veraltete Version mit JetBrains Mono, welche auf meiner Maschine mit Windows11 zu Errors führt. Ist aber ein leichter Fix mit default_profile.dict. Spinbox in source/Widgets.py nutzt noch Segoe. Das wäre vielleicht auch ein guter Moment, die UI-Schriftart dynamisch zu implementieren, als sie überall seperat zu hardcoden. Ich bin zwar kein Fan von der Optik von DejaVu Sans, aber das kann man später immer noch anpasen.
Zu den einklappbaren Fehlermeldungen: Ich sehe noch nicht so ganz den Sinn dahinter. Im Normalfall hat der Nutzer nicht Developer-Mode an und hat somit nur ein paar Zeilen Fehlermeldung. Warum muss man das einklappen? Außerdem habe ich, nach ein paar Tests Bugs gefunden. Beim Ausklappen verschwindet der Fehlertitel rechts vom Pfeil, selbst nach Einklappen.
Den Windows-spezifischen Funktionsaufruf "ctypes.windll.shcore.SetProcessDpiAwareness(1)" mit try einzubetten ist eine sehr gute Lösung. Ich würde das ganz nur in Assemblitor.pyw verschieben, weil das eher zu Setup/Import/Technisches gehört (ich weiß auch nicht, warum ich das ursprünglich in Editor gepackt habe). "Exception" mit "AttributeError" zu ersetzen ist auch Good Practice.
Highlighting für Kommentare ist super. Nur sollte die Farbe nicht gehardcoded sein, sondern in Editor.set_theme() definiert werden. Das Grün ist sehr ähnlich zu dem Grün der aktiven Zeile im Schrittmodus. Evtl. (wie viele IDEs auch) Kommentarfarbe grau machen?
Das Autoshift-Feature klingt super und sehr praktisch, aber bei mir funktioniert es nicht. Enter fügt eine neue leere Zeile hinzu und shiftet gar nichts, aber Löschen einer Zeile direkt im Anschluss shiftet zurück. Wurde vorher keine neue Zeile eingefügt, tut Zeile Löschen auch nichts.
Warum braucht es Fallbacks für eh.error()? Diese Funktion sollte immer funktionieren, und wenn nicht, dann sollte das ein Packimportfehler sein, der schon bei Startup abgefangen wird.
Ich verstehe das Sentiment hinter Zeile 85 in PackHandler.py. Aber so etwas zu hardcoden ist meiner Meinung nach schlechter, als Backwards-Compatability zu erhalten. Ich bin eh dafür, bei jeglichen Import-Fehlern von profile.dict einfach das default_profile.dict rüberzukopieren. Das würde hier beides lösen.
Die neue Option für Autoshift verlängert die Settings, ohne die Dimensionen des Fensters anzupassen. Dadurch wird die (normalerweise unsichtbare) "Neustart erforderlich"–Meldung unter die Buttons am unteren Rand verschoben und wird somit nicht mehr angezeigt. Ich sollte eh mal das Settings-Fenster resizable machen...
Der "New Features"–Abschnitt im README.md kommt eher in der Realease-Beschreibung auf GitHub (und wenn, dann eher unter "## The Editor"). Im Readme sollte statischeres sein, wie z.B. die zwei tollen neuen Befehle: DIV und JNZ!
Zusammenfassung
Diese Pull Request behebt kritische Fehler, die verhindern, dass die Anwendung auf Linux-Systemen ausgeführt wird.
Probleme
ctypes.windllist nur unter Windows verfügbar und verursachte einen Fehler beim Importieren vonEditor.pyBehobene Änderungen
1.
/program/source/Editor.pyctypes.windll.SetProcessDpiAwareness()wird nur noch auf Windows (sys.platform == "win32") ausgeführt