ikskuh / kristall

Graphical small-internet client for windows, linux, MacOS X and BSDs. Supports gemini, http, https, gopher, finger.
https://kristall.random-projects.net/
GNU General Public License v3.0
742 stars 42 forks source link

Bug in manpage creation using dash #274

Closed charles2910 closed 1 year ago

charles2910 commented 1 year ago

Hi, Felix. I'm here again :-)

So Debian links /bin/sh to /bin/dash to interpret commands. The problem is dash doesn't support different locales and it's breaking the sed -E command. Invoking bash explicitily solves the problem.

The problem: Screenshot from 2023-06-01 22-03-09

Using #!/bin/bash: Screenshot from 2023-06-01 22-03-49

Now, is it ok to change the hashbang to /bin/bash? I'm thinking about users outside Debian (like BSD and MacOS)

ikskuh commented 1 year ago

This is actually a good question, i think we actually switched from bash to sh as it wasn't compatible to other systems. is sed a shell builtin in dash?

charles2910 commented 1 year ago

we (sergiodj, samueloph and I) are suspecting the problem is in echo (which is a built in commad), not actually in sed. I'll try to do a debug session during the weekend to pinpoint the issue and report upstream. I thought "maybe I should try to report this quick and dirty fix before", but I see it might be a problem to other systems.

charles2910 commented 1 year ago

Ok, I was able to pinpoint the problem: image

Double escaping seems to solve it in sh/dash, but it put 2 backslashs in bash. I'm not sure what is the correct/expected behavior in this case.

sh/dash:

$ echo $(echo press *Return* and | sed -E -e 's#(^|[.,!? ]+)[*_]([^*_ ]+[^*_]+[^*_ ]+)[*_]($|[.,!? ])#\1\\\\fI\2\\\\fR\3#g')
press \fIReturn\fR and

bash:


charles@x1-carbon:~$ echo $(echo press *Return* and | sed -E -e 's#(^|[.,!? ]+)[*_]([^*_ ]+[^*_]+[^*_ ]+)[*_]($|[.,!? ])#\1\\\\fI\2\\\\fR\3#g')
press \\fIReturn\\fR and
charles2910 commented 1 year ago

Now I think I've found the answer. sh and, consequently, dash interpret some escaped characters as control sequences, so an echo '\f' outputs a form feed. bash's echo defaults to ignore escaped characters unless explicitly told otherwise (-e flag).

So, my question is: should we double escape \f to comply with dash? If you agree, I can open a PR with this fix:

diff --git a/doc/gen-man.sh b/doc/gen-man.sh
index 4463c5a..52b5132 100755
--- a/doc/gen-man.sh
+++ b/doc/gen-man.sh
@@ -59,8 +59,8 @@ gem_in=$(
     # First expression replaces all [Text like this] with bold text.
     # Second expression replaces text like *This* or _this_ with italic text.
     sed -E \
-      -e 's#\[([^]]*)\]#\\fB\1\\fR#g' \
-      -e 's#(^|[.,!? ]+)[*_]([^*_ ]+[^*_]+[^*_ ]+)[*_]($|[.,!? ])#\1\\fI\2\\fR\3#g'
+      -e 's#\[([^]]*)\]#\\\\fB\1\\\\fR#g' \
+      -e 's#(^|[.,!? ]+)[*_]([^*_ ]+[^*_]+[^*_ ]+)[*_]($|[.,!? ])#\1\\\\fI\2\\\\fR\3#g'
 )

 # Convert gemtext to man format
ikskuh commented 1 year ago

Yeah, can you try make a PR?

charles2910 commented 1 year ago

Yeah, of course!

charles2910 commented 1 year ago

I think we can close this issue, right?