Bugfix: core hookspath

[gabyx]

The path to --use-core-hookspath is not parsed and set. This is a small bug fix, with some minor cosmetics.

• Less ifs, less early returns (never good to understand)
• Some errors in function mark_directory_as_target.

P.S: Where I am not sure, but nothing to this PR. After step # 4. Setup new folder if running in find_git_hook_templates I am not sure what happens if we are there and --use-core-hookspath. I think its fine. Its a bit confusing... But in another way its also clear you choose a template dir and then it gets used as the core.hooksPath...

[coveralls]

Pull Request Test Coverage Report for Build 488

• 32 of 45 (71.11%) changed or added relevant lines in 2 files are covered.
• 17 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-1.2%) to 86.622%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
install.sh	25	38	65.79%
<!--	Total:	32	45	71.11%	-->

Files with Coverage Reduction	New Missed Lines	%
cli.sh	17	88.08%
<!--	Total:	17		-->

Totals
Change from base Build 487:	-1.2%
Covered Lines:	1826
Relevant Lines:	2108

---

💛 - Coveralls

[gabyx]

Lets test.

[gabyx]

All test pass.

[rycus86]

I'll check this PR later today. Thanks!

[rycus86]

Hm, I'm a bit confused about this PR, what is the fix actually in here? I see that you rewrote a bunch of ifs and return statements (most of which I disagree with :) ), changed some braces around variable names, but the logic doesn't seem to change.

If there is a bug, could you please have the PR fix that only? If you'd like to discuss those code style/program flow changes, I'm open to it, but let's talk about them on an issue maybe first (maybe a PR is better to comment on the code itself), but the important thing is that we make that issue/PR about the styling changes, not the bugfix - hope that sounds OK?

[gabyx]

Clear. Was a bit frustrated, with bash and IMO strange code flow latetly ... I comitted only the relevant stuff.

The bug fixes is in install.sh

• L3686, which is not right. (according to the function what it should do)
• $ sh -c "$(curl -fsSL https://r.viktoradam.net/githooks)" -- --use-core-hookspath /home/public/.githooks does not work -> Fixed it. L3501
• The control flow in find_git_hook_templates has some errors after step 4 when --use-core-hookspath is used and we reach to step 4 in this function. Think about it. My fix is what should be done to set the correct paths, however my fix doesnt make the control flow better.
• Added a another warning on L4305. If the user messes up (like me :-P)

Feel free to incorporate what you think is valuable I came across this when trying to experiment with install.sh in a script on the server... And I had also some mess with the state and such...

[rycus86]

Sure, no probs and thanks for the changes! I'll have another look at this shortly.

[gabyx]

Ok, now its correct.

[gabyx]

Wua, thats hard to grasp controlflow, damm, now its better. :-|

[gabyx]

@rycus86 : I added a test 112 which is simple. It does currently not work on master, but it should in my opinion. Are you not also the opinion, that my changes (not much) might go into the right direction? At least it fixes the test. Also I checked again: Reaching step 4 with --use-core-hookspath is also perfectly fine. # 4. Setup new folder if running interactively and no folder is found by now. Maybe you have some time to have a look.

I would also suggest to have one place to call set_githooks_directory (I left that intentionally out) by distinguising between --use-core-hookspath and not... instead of three places, also deep down the callstack ;-).

[rycus86]

Thanks for the test! I left some comments on it, that it looks like it's more a test setup issue rather than an install script issue. Still, I don't quite get why are there corePath related changes in this PR if the problem is with the template directory? (if I understand correctly)

[rycus86]

For this one

$ sh -c "$(curl -fsSL https://r.viktoradam.net/githooks)" -- --use-core-hookspath /home/public/.githooks does not work -> Fixed it. L3501

I see what you mean now. It feels like a much cheaper fix would be:

• git config --global core.hooksPath $argFromShell
• do the existing logic

[rycus86]

What do you meant by this, sorry? (I think line numbers have got messed up)

L3686, which is not right. (according to the function what it should do)

[rycus86]

Please check out https://github.com/rycus86/githooks/pull/69 , I think this is not actually an issue, but the README was quite misleading on the parameter usage.

[gabyx]

One question: When a valid existing init.tempalteDir is set but no hooks subdirectory. A normal installation should work with out specifying --template-dir. (shouldn't it?)

I will update accordint to the PR #69 .

[rycus86]

In that case, you template dir might not have been valid, and I think the installer will continue trying the default location in /usr/..., then offer to search for it, then offer to create one (by default at ~/.githooks/templates (no hooks subfolder in this one).

Sorry about the back and forth in this PR, I couldn't remember these things until I started writing those tests last night, then ot clicked that it should be working, only the README is wrong.

[gabyx]

Closed in favor of a new one, to restart again.