soveran / clac

Command-line, stack-based calculator with postfix notation
BSD 2-Clause "Simplified" License
357 stars 30 forks source link

Suggestion: Comments in words file #25

Open cjpbirkbeck opened 3 years ago

cjpbirkbeck commented 3 years ago

Hello, I have been using this program for a bit and I have noticed that the words file doesn't allow for comments. I think have some comments would useful even for a small words file.

Here's my proposal:

I'm not exactly the world's greatest C programmer, but I would be willing to try doing it if you think it's a reasonable idea.

soveran commented 3 years ago

Hey @cjpbirkbeck, I think it is a good idea. I have been using # as an alias of count, but I will happily give it away in order to have comments. The patch is simple:

diff --git a/clac.c b/clac.c
index 7d08fa5..3fce9d2 100644
--- a/clac.c
+++ b/clac.c
@@ -260,6 +260,10 @@ static void load(sds filename) {
    for (i = 0; i < linecount; i++) {
        lines[i] = sdstrim(lines[i], " \t\r\n");

+       if (lines[i][0] == '#') {
+         continue;
+       }
+
        if (parse(lines[i]) != 0) {
            sdsfreesplitres(lines, linecount);

Before making a definitive decision, I would like to have the opinion of @waltertross, another big user of clac.

waltertross commented 3 years ago

hi, it would work, but it's a bit of a pity if the # cannot be used as a word any more. I thought of //, but that can also be useful as a word for integer division (if you come from Python). I don't know, maybe ' or ". Also /* closed by */ would be nice, but spreading this over lines, or handling more than one on a line, would make the code definitely more complicated. An idea which would break lots of rules would be a /* at start of line, NOT CLOSED. But that's too revolutionary, probably ;-) A very conservative idea, OTOH, would be to NOT allow line comments, and to allow a comment string (enclosed in quotes) to follow the word definition string. This would stay associated with the word, and would be printed out by the words command.

wtross-eg commented 3 years ago

Ah, but in search for a solution I found that the # at start of line is perfectly viable, because already now the word being defined can be enclosed in double quotes, so that your alias for count, @soveran, can be defined as "#"! So... green light!

waltertross commented 3 years ago

Woops, I left my last comment as wtross-eg. It's still me, but as an EG employee...

soveran commented 3 years ago

Wow, that's a great finding! I tested it and having comments looks like a good improvement. I'll document it and push the changes.

soveran commented 3 years ago

Added in this commit: https://github.com/soveran/clac/commit/ba208c0380bae7acb79d7fd896c2b0d67ea3205d

@waltertross @cjpbirkbeck If you are OK with the implementation and the comment, I will release a new version.

waltertross commented 3 years ago

Maybe for completeness you might want to add something similar to the following, somewhere:

Note that to define the word # you have to use "#" in order for the definition not to be interpreted as a comment

If you don't write something like this anywhere, most users will be unable to define the # word.

cjpbirkbeck commented 3 years ago

Maybe for completeness you might want to add something similar to the following, somewhere:

Note that to define the word # you have to use "#" in order for the definition not to be interpreted as a comment

If you don't write something like this anywhere, most users will be unable to define the # word.

I agree and I would suggest that an extra paragraph is added to the man page documenting how comments are written and how to escape # . Again, I'm not sure if I have written the troff syntax correctly, but I would suggest something like the following:

.
.Ss Comments
.
Any lines that begin with
.Sy #
are considered to be a comment, and are ignored. There are no inline comments, and any 
.Sy #
characters that is not the first character is interpreted literally. To assign 
.Sy #
as an operation, use 
.Sy "#"
. So, for example:
.Pp
.Dl # This is a comment.
.Dl "#" "count"
.Pp
This assigns
.Sy #
to the operation
.Sy count.
.
soveran commented 3 years ago

Excellent, I added a paragraph to both the README and the man page: https://github.com/soveran/clac/commit/beae8c4bc89912f4cd66bb875585fa471692cd54

Can you check it?