cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.76k stars 176 forks source link

Avoid exporting HISTFILE #308

Closed cantino closed 1 year ago

cantino commented 1 year ago

Hopefully fixes 249.

cantino commented 1 year ago

@dithpri (or @AndrewKvalheim, if you're open to it... you've already suffered enough), would you be able to test this?

dithpri commented 1 year ago

From the quick testing I've done (the repro described in 249), this works. I think this is also much better than the solution I had suggested -- and obviously forgot about, for which I apologize -- since it avoids exporting HISTFILE completely, not even as MCFLY_HISTFILE.

From a glance at the code, this could break compatibility with fish, as mcfly.fish wasn't updated to not use MCFLY_HISTFILE. I have however no experience with fish and just running it with the default mcfly setup has produced no errors. Ok, so I think the fish script uses a different way of adding commands. Is MCFLY_HISTFILE still needed there?

cantino commented 1 year ago

Thanks @dithpri!

I'm honestly not 100% sure if fish needs it anymore. The only code path that seems like it'd hit it for fish would be calls to shell_history::history_file_path, and then only if HISTFILE isn't set.