Skip to content

Refactor readlink_f to common helpers#366

Closed
OJFord wants to merge 5 commits intotfutils:masterfrom
OJFord:refactor-readlink
Closed

Refactor readlink_f to common helpers#366
OJFord wants to merge 5 commits intotfutils:masterfrom
OJFord:refactor-readlink

Conversation

@OJFord
Copy link
Contributor

@OJFord OJFord commented Nov 5, 2022

In the course of fixing #355's use of realpath --relative-to for macOS, I tried to use readlink --canonicalize/-f which had a similar problem, and through that noticed the extensive extant use of a readlink_f function to achieve that functionality. The same function is currently replicated 23 times through the repo.

In the interest of readability, maintainability etc., this PR refactors that to the common helpers.sh, exporting it for use everywhere without redefinition.

#355 also revealed a bug in the implementation I copied again, which I subsequently fixed in that instance. That's reproduced here too (and thus for all uses of the function) in 1784a4f.

(This is based on top of #355, since that PR adds another instance of this function. I'll rebase this once that's merged - please consider this a draft until then.)

@OJFord OJFord force-pushed the refactor-readlink branch from 1784a4f to 7df205d Compare November 5, 2022 00:20
@OJFord
Copy link
Contributor Author

OJFord commented Nov 5, 2022

Bah, I see why this was done now, the existing use of readlink_f is in finding TFENV_ROOT, and helpers.sh is sourced from within that dir.

@OJFord OJFord closed this Nov 5, 2022
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.

1 participant