Skip to content

Added support for header keyword argument #42

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 11, 2025
Merged

Added support for header keyword argument #42

merged 4 commits into from
Mar 11, 2025

Conversation

fearphage
Copy link
Contributor

Just converted another CLI param to a keyword argument.

dahlia
dahlia reviewed Mar 10, 2025
View reviewed changes
Copy link
Owner

@dahlia dahlia left a comment

Choose a reason for hiding this comment

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

Could you write docs for this new option in README.rst? Thanks!

@fearphage
Copy link
Contributor Author

@dahlia Do you mind if I clean up the readme a bit? I was thinking of turning the options into a sorted table to make finding entries more straight forward.

@dahlia
Copy link
Owner

dahlia commented Mar 10, 2025

@dahlia Do you mind if I clean up the readme a bit? I was thinking of turning the options into a sorted table to make finding entries more straight forward.

Sure! That sounds a good idea.

fearphage
fearphage commented Mar 10, 2025
View reviewed changes
README.rst
-
- The iterable of extra raw options/arguments to pass to ``fzf``.

This is how you pass extra options that are not already defined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this sentence.

@fearphage
Copy link
Contributor Author

@dahlia Good to go. I added a note about #40 and added docs for the header param. Most of the rest is just reformatted/cleaned up copy and paste from the previous readme.

One thing of note that I didn't find a quick fix for. The "API reference" link in the readme doesn't work in the main branch. I was going to fix it here, but I didn't quickly find out how to resolve that in the RST format. If you know what's wrong, I'd be happy to fix it while I'm here.

@fearphage fearphage requested a review from dahlia March 10, 2025 22:52
dahlia
dahlia approved these changes Mar 11, 2025
View reviewed changes
Copy link
Owner

@dahlia dahlia left a comment

Choose a reason for hiding this comment

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

Thanks for reformatting docs!

@dahlia dahlia merged commit b42c920 into dahlia:main Mar 11, 2025
4 checks passed
@fearphage fearphage deleted the add-header-kwarg branch March 11, 2025 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dahlia dahlia dahlia approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fearphage @dahlia