-
Notifications
You must be signed in to change notification settings - Fork 19
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
add header and color options #12
base: main
Are you sure you want to change the base?
Conversation
This would be a great feature. I'd love to help get it merged but I can't tell what is failing in the CI. Any help @dahlia ? |
I see a message about python 2 usage on the beggining of the console output. Maybe this has something to do with it? |
I'm guessing the python2 tests are failing with the f-string syntax. Try replacing: |
I just replaced this part but the build still fails. That's weird |
this one looks like a different error. i defer to @dahlia who probably has a better understanding of the CI setup. but the syntax error from before is gone and now we just have:
|
@mvrozanti Sorry for my late response. As @daturkel explained, it was an error that has existed before changes you made, and I finally fixed the error in the master. The error would be gone if you rebase your commits on the current master. |
Still failing 😕 . I actually just tried pulling from upstream. Do I really need to rebase? |
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.
-
Please make its style consistent with the existing code. You can run the lint to check style:
tox -e lint
(you should installtox
first:pip install tox
). FYI the CI build is failing due to style inconsistencies:iterfzf\__init__.py:55:70: E231 missing whitespace after ',' iterfzf\__init__.py:55:80: E501 line too long (91 > 79 characters)
-
Please write changelogs in the README.md file.
@@ -29,7 +29,7 @@ def iterfzf( | |||
# Search mode: | |||
extended=True, exact=False, case_sensitive=None, | |||
# Interface: | |||
multi=False, mouse=True, print_query=False, | |||
multi=False, mouse=True, print_query=False, header=None, color=None, |
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.
Could you move header
parameter to # Layout:
section? It looks more appropriate there.
@@ -1,4 +1,4 @@ | |||
from typing import AnyStr, Iterable, Optional | |||
from typing import Dict, AnyStr, Iterable, Optional |
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.
from typing import Dict, AnyStr, Iterable, Optional | |
from typing import AnyStr, Dict, Iterable, Optional |
Imports should be listed in lexicographical order.
8941643
to
ff4761d
Compare
No description provided.