-
-
Notifications
You must be signed in to change notification settings - Fork 13
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: SyntaxError #48
base: master
Are you sure you want to change the base?
fix: SyntaxError #48
Conversation
Thanks for this. |
Unfortunately I don't have my computer for a month or 2 so I can't rebase it for now |
Maybe closing/reopening will retrigger CI? |
CI on travis should probably be disabled, it costs "credits" to run on x86_64 |
CI is passing, where it can |
1f4caa5
to
7ed1044
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently code formatting was also applied, likely using black?
I think only fixing errors might be better / easier to review, but am not opposed to have it as it is now.
this one https://github.com/akaihola/darker
i dont know style used on this repo so i used darker formatter as default if there is error due to code style i will remove it from pr and make the minimal change |
pyrepl/keymaps.py
Outdated
@@ -77,8 +77,7 @@ | |||
(r"\<home>", "home"), | |||
(r"\<f1>", "help"), | |||
(r"\EOF", "end"), # the entries in the terminfo database for xterms | |||
(r"\EOH", "home"), # seem to be wrong. this is a less than ideal | |||
# workaround | |||
(r"\EOH", "home"), # seem to be wrong. this is a less than ideal workaround |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it appears with "home" only, but likely is also meant for "end". (just from what I think/see, no idea really).
I wanted to ensure that the comment's meaning/location/reference is kept (which was given before due to whitespace alignment), but now it is even less clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya I agree. It looks like the comment spans multiple lines and starts with "the entries in the terminfo database.." IMO it should be on a separate line, before the end & home lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0262f36 fix that
should there be new line between help
and comment? if yes, i will create another commit for that
@blueyed is this ready for merge? |
i think it is ready some suggestion already added to pr |
Proposed changes
Types of changes
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...