refactor(HISHTORY_PATH)!: Relative to absolute.

[dezza]

Removes relative path logic for $HISHTORY_PATH and instead expands $HOME

Gets $HOME from golang/os os.UserHomeDir() instead of ctx. Deprecates old helper function, reverts temporary guard-conditional.

#276

BREAKING CHANGE: Users need to update their $HISHTORY_PATH env variable.

[GRbit]

Thanks for your work on this!

Yet, one thing to consider, how about backward compatibility? It looks like this change will break existing setups.
A possible approach to support both could be to check whether the path in HISHTORY_PATH is absolute. If it's not absolute, prefixing it with the user's home directory can help maintain compatibility. And it would be nice to have tests to verify that the old users will not be affected by any changes in the future.
What do you think?

[dezza]

Yet, one thing to consider, how about backward compatibility? It looks like this change will break existing setups. A possible approach to support both could be to check whether the path in HISHTORY_PATH is absolute.

I prefixed this commit with ! to indicate breaking change (major version-ish bump) as per conventional commit.

In this case I think using relative paths for this is simply too far away from what the norm is with env variables for shell tools - its all absolute. Having relative path in the first place is not that useful. If you insist on having a relative path then just expand it in HISHTORY_PATH="$(realpath "relative_dir")". I don't think it should be a reason to needlessly add complexity to the code. It is a very uncommon approach.

See #276

[GRbit]

@dezza, I think I understand your perspective, and I want to clarify I’m not suggesting that we should actively encourage the use of relative paths. My main concern is the existing users – this project has been around for two years and has an established user base.
If someone is already using this environment variable and updates the app (especially since there’s a built-in command to facilitate updates), they might suddenly find their setup broken. That will lead to a frustrating user experience.

I believe that if we can maintain backward compatibility without significant overhead or efforts, that's the way. What do you think?

[dezza]

I believe that if we can maintain backward compatibility without significant overhead or efforts, that's the way. What do you think?

I don't think its a good idea. It would be better to warn people if attempting to use a relative path after update.

[GRbit]

That sounds interesting. Warning is way better than just breaking it.

Personally, I'd vote for backward compatibility, but I'll be genuinely happy if it will not break someone's experience silently. That would be very nice improvement, thank you for sharing it.

[dezza]

That sounds interesting. Warning is way better than just breaking it.

Personally, I'd vote for backward compatibility, but I'll be genuinely happy if it will not break someone's experience silently. That would be very nice improvement, thank you for sharing it.

But is there a future for that backward compatibility "feature"? Did you see how much repetitive code it removed regarding homedirs?

The fact it had to be relative path puzzled me from the very start. It wasn't immediately obvious and opposite of common practice regarding path for ENV vars. There is no purpose either, since its not supposed to be portable like a Makefile project, where I could see relative paths being used, for that reason alone.

Relative / Absolute path is equally valid for XDG_CONFIG_HOME settings, but the former is odd compared to how the rest of shell variables are set to absolute.

[GRbit]

But is there a future for that backward compatibility "feature"?

I hope there isn't, and that no one will use it. But what does have a future is the project itself and its reputation. Sometimes when I make libraries or applications, I make mistakes. In my experience, when building libraries or applications, I sometimes make mistakes. But once users (if we're lucky enough to have them) start relying on the code, even if it's not ideal, it becomes part of their workflow. I value my users, I respect the time they spent adapting to my mistakes, so if I can maintain backwards compatibility, I do so. And I value projects which respects this.

That said, I really value your contribution, you are doing it for free and deserve gratitude. I also appreciate this conversation; I’m just another user who cares about the project, not a maintainer. My opinion about the code is that if it helps users convenience – it makes sense to me. We read the code occasionally, but we use it daily. If I have too many lines of repetitive code, I just make a function.

Also, I want to note that I don’t have any more authority over this project than you do, I’m just another happy user. If you feel strongly that this change should go through, and it'll benefit the project, I completely respect that.

[dezza]

Sure thing, no hard feelings :) I value new eyes on all the contributions I make and its just as inspiring to me.

In this particular case I think it was the wrong choice from the start, so minimizing the additional code that is leaning into the anti-pattern I think is way forward to that breaking change. I still think a check + warning if you use a relative path forward is good user experience until its been the standard for years, instead of cutting the cord and just be like "read the changelog or expect it to not work one day".

Whether it will only be merged later on a major version bump is fine by me, even if the project is new and has many users its still relatively early, what I would consider alpha.