xmonad-contrib
xmonad-contrib copied to clipboard
X.P: Add escape hatch for preventing X.P IO
Rationale
When using XMonad.Prompt for multiple (8+) different use cases in my configuration, I never once needed a command history.
The default of reading and writing a file for the command history opaque to the user is strange to me, and each prompt is sharing and replacing the same prompt-history file... but I digress.
My feature request is to stop that behavior, where historySize = 0 didn't cut the mustard since prompt-history is still reading and writing on every prompt.
Proposal
This pull request adds historyRead and historyWrite configuration options to XPConfig, allowing users to write something like:
...
, historyRead = \cacheDir -> return empty
, historyWrite = \history cacheDir -> return ()
...
This could open up functionality for using other state for a command history, but maybe that isn't desired.
My main priority with this pull request is to prevent the reading and writing of the prompt-history file, and as such I am marking this as a draft until discussion resolves the best path forward.
Alternative proposal (not implemented)
historySize = 0 could branch the behavior, and prevent any related IO (reading or writing).
Checklist
-
[x] I've read CONTRIBUTING.md
-
[x] I've considered how to best test these changes (property, unit, manually, ...) and concluded: unknown, I am using these changes locally
-
[ ] I updated the
CHANGES.mdfile
Maybe a better path forward is to remove the prompt-history writing behaviour by default in a breaking change, and then supporting users to opt-in by adding this kind of configuration?
...
, historyRead = readPromptHistory "prompt-history"
, historyWrite = writePromptHistory "prompt-history"
...
@slotThe historyCompletionP needs conf either way, but yes branching as I mentioned is an alternative approach
historySize = 0 could branch the behavior, and prevent any related IO (reading or writing).
I'm happy to implement that if the alternative isn't preferable
@slotThe I have changed the pull request to pattern match historySize = 0 for readHistory and writeHistory
@slotThe the only downside to a non-breaking change is that historyCompletionP, if used, will not respect the new behaviour (for better or worse), I think this is the right path for now though
Both historyCompletion and historyCompletionP aren't very widely used within contrib, so I would even be open for a breaking change there, perhaps making them take an XPConfig (or even just the historySize) as an additional argument. I think the number of (actually useful) custom prompts using these in the wild is relatively low
@dcousens ping for ^
@slotThe to clarify, you think I should introduce a breaking change [that you have suggested] in this pull request? Assuming yes, I'll try and do that this weekend
@dcousens sorry for only getting back to you now: I'm saying that I feel ambivalent about introducing a breaking change for historyCompletion[P]; these functions probably aren't very widely used, and the uses they have inside of contrib are all in the vicinity of an XPConfig argument. However, if you feel otherwise them I'm also fine with merging this as-is
OK, I'll have a shot at that, I'd prefer that personally
@dcousens friendly ping :)
Rebased and added a breaking commit, which ensures that historyCompletion* now accepts an XPConfig
Thank you!