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

add OILS_COMP_UI #2244

Merged
merged 6 commits into from
Feb 3, 2025
Merged

add OILS_COMP_UI #2244

merged 6 commits into from
Feb 3, 2025

Conversation

melvinw
Copy link
Collaborator

@melvinw melvinw commented Feb 2, 2025

This will be more user-friendly than the existing command-line arg, since people can set it in their init files.

@melvinw melvinw force-pushed the melvinw/comp_ui_env branch from 05e7e2e to 31801a3 Compare February 2, 2025 16:46
@andychu
Copy link
Contributor

andychu commented Feb 2, 2025

OK I think this is a good change

We should document it in:

  • doc/ref/toc-osh - maybe near OILS_VERSION, add an [Interactive] section that lists OILS_COMP_UI
  • doc/ref/toc-ysh - ditto - applies to both OSH and YSH
  • doc/ref/chap-special-var.md - there should be a ## Interactive, and then ### OILS_COMP_UI

and I would mention that it is only consulted at startup

(and then we can also document OILS_MAX_COMP_LINES in the same place)


It is possible to write a test for this? I guess it is hard to test because it changes the TAB behavior only ...

I suppose the only way we have of testing that is spec/stateful, which uses pexpect

I guess we are going to run into the same issue with OILS_MAX_COMP_LINES - it is hard to test

hmmmm

@melvinw
Copy link
Collaborator Author

melvinw commented Feb 2, 2025

Pushed some docs. Let me know if I should tweak the wording

Copy link
Contributor

@andychu andychu left a comment

Choose a reason for hiding this comment

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

Looks good!

### OILS_COMP_UI

Set which completion UI to use. Set to `minimal` for a UI that approximates the
default behavior of readline. Set to `nice` for a UI with a fancy pager and a
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say "GNU readline" because some people might not know what "readline" is

I think it will nicer to read with a bulleted list, like

  • minimal - ...
  • nice - ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 done

@andychu andychu merged commit 847984b into soil-staging Feb 3, 2025
18 checks passed
@andychu
Copy link
Contributor

andychu commented Feb 3, 2025

Great thanks!

I pushed a test/manual.sh for now ... I am wondering about this ENV.OILS_COMP_UI issue for YSH, which I started doing in November or so ...

( I am trying to untangle the confusion of global vars in shell ... which ones are from ENV and which ones aren't? There are a bunch of quirks around PWD, PS1, etc. )

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.

2 participants