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

streampager: allow running Streampager without reading its config files #1011

Closed
wants to merge 1 commit into from

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Jan 15, 2025

This could be considered as a bug report/feature request instead of a PR.

Currently, there is no way for a calling program (like sl or jj) to start streampager without it trying to read
$CONFIG/streampager/streampager.toml and the environment variables SP_INTERFACE_MODE, SP_SCROLL_PAST_EOF, and SP_READ_AHEAD_LINES. This is undesireable if we'd like to control Streampager's behavior fully from jj config (or sl config). Sapling currently overrides the most important Streampager config from its own config anyway.

I'd like there to be a way to invoke Streampager with a given config. I carefully implemented it so that no changes to Sapling are required, though I think it'd also benefit from using the API I introduce here (and the old UI could be deprecated).

If there's a better way to achieve that than my approach here, I'd be happy to do that. Another, less invasive, possibility would be to simply make Pager::config public. However, this would mean that Streampager would uselessly read $CONFIG/streampager/streampager.toml even if the config is later fully overriden by jj.

As an aside, the docs for the config from
https://github.com/markbt/streampager#example-configuration do not seem to be fully correct, setting interface_mode = "delayed" as described there did not work for me. However, I'd rather not use that configuration method at all for my purposes with jj.

Cc: jj-vcs/jj#4203 (comment), @yuja.

As ever, thanks for making it convenient for us to use Streampager in jj! :)

@facebook-github-bot
Copy link
Contributor

Hi @ilyagr!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@ilyagr ilyagr marked this pull request as ready for review January 15, 2025 02:09
@ilyagr ilyagr changed the title streampager: provide functions to run Streampager without reading config files streampager: allow running Streampager without reading its config files Jan 15, 2025
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.)

@facebook-github-bot
Copy link
Contributor

@ilyagr has updated the pull request. You must reimport the pull request before landing.

Currently, there is no way for a calling program (like `sl` or `jj`) to
start streampager without it trying to read
`$CONFIG/streampager/streampager.toml` and the environment variables
`SP_INTERFACE_MODE`, `SP_SCROLL_PAST_EOF`, and `SP_READ_AHEAD_LINES`.
This is undesireable if we'd like to control Streampager's behavior
fully from `jj` config (or `sl` config). Sapling currently overrides
the most important Streampager config from its own config anyway.

I'd like there to be a way to invoke Streampager with a given config.
I carefully implemented it so that no changes to Sapling are required,
though I think it'd also benefit from using the API I introduce here
(and the old UI could be deprecated).

Another, less invasive, possibility would be to simply make
`Pager::config` public. However, this would mean that Streampager
would uselessly read `$CONFIG/streampager/streampager.toml` even
if the config is later fully overriden by `jj`.

As an aside, the docs for the config from
https://github.com/markbt/streampager#example-configuration do not seem
to be fully correct, setting `interface_mode = "delayed"` as described
there did not work for me. However, I'd rather not use that
configuration method at all for my purposes with `jj`.

Cc: jj-vcs/jj#4203 (comment),
@yuja.

As ever, thanks for making it convenient for us to use Streampager in
`jj`! :)
@facebook-github-bot
Copy link
Contributor

@ilyagr has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 26edb15.

@ilyagr
Copy link
Contributor Author

ilyagr commented Jan 27, 2025

Thank you very much!

@ilyagr ilyagr deleted the with_config branch January 27, 2025 23:15
@ilyagr ilyagr restored the with_config branch January 27, 2025 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants