Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/windows compatibility #165

Conversation

razaabbasnextgen
Copy link

@razaabbasnextgen razaabbasnextgen commented Oct 7, 2024

Important

Fix Windows compatibility by conditionally importing termios in chat.py and commands.py.

  • Compatibility:
    • Conditional import of termios in chat.py and commands.py for non-Windows platforms to ensure compatibility.
  • Misc:
    • Add placeholder chat() function in chat.py and commands.py.

This description was created by Ellipsis for 5f57f4c. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 5f57f4c in 22 seconds

More details
  • Looked at 57 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. gptme/commands.py:25
  • Draft comment:
    The chat function defined here is redundant and should be removed.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_rhFN59yCrqtvQLNd


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

if sys.platform != 'win32':
import termios

def chat():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chat function defined here is redundant and should be removed.

@brayo-pip
Copy link
Collaborator

You would also need to add conditional code for windows everywhere that sys is called. Currently this does not do that

@ErikBjare
Copy link
Owner

ErikBjare commented Oct 8, 2024

This diff looks messy, and doesn't come close to fixing the entire issue of native Windows/PowerShell support.

Closing due to predicting it will go stale.

@ErikBjare ErikBjare closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants