olets / zsh-abbr

The zsh manager for auto-expanding abbreviations, inspired by fish. ~13,000 unique cloners as of May '24, 580+ Homebrew installs 6/23-6/24
https://zsh-abbr.olets.dev
Other
511 stars 18 forks source link

support setting ABBR_TMPDIR to a value that doesn't end in `/` #100

Closed ealap closed 2 months ago

ealap commented 9 months ago

Changes:

Tested on Debian and MacOS.

ealap commented 9 months ago

Thanks for checking this. I changed the PR to focus on improving the handling of trailing slash to ABBR_TMP value. I'll create a separate PR for handling issue #54.

One question: How do I update this?

If we make this change, we'll also need to update the documentation

olets commented 9 months ago

Thanks for making that update!

--

How do I update the documentation?

The repo is https://github.com/olets/zsh-abbr-v5-docs. There's also a direct "edit" link to the file at the bottom of every page on the docs site.

--

I apologize: Looking at this again, I would prefer the opposite of what I said in https://github.com/olets/zsh-abbr/pull/100#discussion_r1332109316. If we make ABBR_TMPDIR always end in /, then

--

But before spending time on additional changes to this or to the docs, can you describe the problem you're solving?

I get that the "end custom ABBR_TMPDIR values in a /" is a gotcha. But I'm wary of changing it. I think the change is safe, but it feels like there might be some edge case. Not saying no, I just want to keep thinking about it for a bit.

ealap commented 9 months ago

I get that the "end custom ABBR_TMPDIR values in a /" is a gotcha.

It was indeed a gotcha moment for me that got me to suggest this change. I tried giving the ABBR_TMPDIR a custom value (based on suggestion from #54) without reading the documentation for it, and discovered it created weird directories like:

/tmp/zsh-abbr-ealapregular-user-abbreviations
/tmp/zsh-abbr-ealapglobal-user-abbreviations

And I think making it always end / could make it look ambiguous on scripts that do not conventionally use ${} for variables.

$ABBR_TMPDIRmycustomdir

But my goal here is to just make this variable accept both values with or without trailing /. I want to use the variable without second guessing if I forgot to put a trailing / or not.

olets commented 8 months ago

make ABBR_TMPDIR always end in /

scripts that do not conventionally use ${} for variables.

$ABBR_TMPDIRmycustomdir would not only look funny, it wouldn't work: it's a single variable with that whole long name. ${X}y isn't a stylistic choice, it's the way to distinguish between a variable Xy and a variable X followed by the string y. In this case, not using ${} is user error.

I tried giving the ABBR_TMPDIR a custom value (based on suggestion from https://github.com/olets/zsh-abbr/issues/54) without reading the documentation for it, and discovered it created weird directories

Makes sense. I'm going to keep thinking about this. Working on several other things right now so it might be a while. Silence doesn't mean I'm ignoring the proposal 👍

olets commented 1 month ago

Hi @ealap thanks for your patience. I've merged your work trailing slash work, and released in it v5.6.0

(v5.6.0 also includes a solution to #54)

Lmk if you'd like me to add you to the list of contributors in https://github.com/olets/zsh-abbr#community and https://zsh-abbr.olets.dev/community/. If so I'll have the all-contributors bot open a pull request. I'll tag you in it, and you'll need to review to make sure the bot pulled the right info