reconquest / shdoc

Documentation generator for shell scripts (bash, sh, zsh). Javadoc for shell scripts.
MIT License
294 stars 63 forks source link

Adding "dot" to the BASH function definition #28

Closed kigster closed 3 years ago

kigster commented 3 years ago

Splitting a large PR into smaller ones — this one adds "." (dot) to the list of allowed characters in a BASH function name, since function names like git.squash are allowed.

seletskiy commented 3 years ago

@kigster: Thanks! Tests are failing, however, could you check? — https://github.com/reconquest/shdoc/runs/1651714739#step:4:135

kigster commented 3 years ago

Yup, looking.

kovetskiy commented 3 years ago

You can't pass more than one argument for shebang @seletskiy

ср, 6 янв. 2021 г., 13:55 Stanislav Seletskiy notifications@github.com:

@seletskiy commented on this pull request.

In shdoc https://github.com/reconquest/shdoc/pull/28#discussion_r552505711:

@@ -1,5 +1,6 @@ -#!/usr/bin/gawk -f

+##!/bin/sh

I do not have access to the Mac environment, but shouldn't #!/usr/bin/env -S gawk -f be sufficient here?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/reconquest/shdoc/pull/28#pullrequestreview-562567149, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACAN7ZH577D6US5FZB6CIODSYQ6S7ANCNFSM4VVZX6CQ .

seletskiy commented 3 years ago

@kovetskiy you can by using -S argument for env:

       -S, --split-string=S
              process and split S into separate arguments; used to pass multiple arguments on shebang lines
kigster commented 3 years ago

Not sure about -S and how portable that is.

The beauty of shdoc is that it's an AWK script. But given that it requires gawk, which gets installed all over the place AND gawk in the shebang doesn't work without -f, AND #!/usr/bin/env gawk -f does not seem to work on Linux (works on OS-X), it made sense to do some research and this is how I discovered the /bin/sh solution.

This is the only way I was able to make shdoc work across all of my test systems: my Mac OS-X laptop, Ubuntu-based Docker container, and the CI. Having read the stack exchange answer, it occurred to me that the /bin/sh solution works across all systems and is probably the most portable solution of them all.

hyperupcall commented 3 years ago

Is there a blocker for this? I applied the updated, original patch, and removed the changes to the shebang and the script began to work with function names with dots, with all tests passing:

image

Is it possible to just merge in those changes to the regex to work with functions with dots? It seems the shebang is being addressed (and discussed) again in PR #35, and adding these changes one at a time, one per PR/issue might be a good idea

kovetskiy commented 3 years ago

Closing in favor of #36