Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: jstrdecode -Q is not enclosing the output in double quotes #12

Closed
1 task done
lcn2 opened this issue Sep 9, 2024 · 17 comments
Closed
1 task done

Bug: jstrdecode -Q is not enclosing the output in double quotes #12

lcn2 opened this issue Sep 9, 2024 · 17 comments
Labels
bug Something isn't working

Comments

@lcn2
Copy link
Contributor

lcn2 commented Sep 9, 2024

Is there an existing issue for this?

  • I have searched for existing issues and did not find anything like this

Describe the bug

missing leading double quote

The output of the jstrdecode(1) with -Q is incorrect:

$ jstrdecode -Q hello
hello"

The leading double quote is missing.

multiple args missing leading double quote and contains stray double quotes

This is also incorrect:

$ jstrdecode -Q one two three
one"two"three"

The leading double quote is missing and the double quotes in the middle are wrong.

What you expect

The result of:

jstrdecode -Q hello

should be enclosed in double quotes:

"hello"

Moreover:

The result of:

jstrdecode -Q one two three

should be enclosed in double quotes without intermediate double quotes:

"onetwothree"

Environment

- OS: yes
- Device: null
- Compiler: c

bug_report.sh output

n/a

Anything else?

The help message for jstrdecode -h is correct:

	-Q		enclose output in quotes (def: do not)

p.s.

We recommend that when jstrdecode(1) is fixed in terms of -Q, the test suite be updated to verify that -Q processing is correct.

@lcn2 lcn2 added the bug Something isn't working label Sep 9, 2024
@lcn2 lcn2 changed the title Bug: jstrdecode -Q does not encode the string in double-quotes, man page is misleading Bug: jstrdecode(1) man page contains misleading text about -Q Sep 9, 2024
@lcn2 lcn2 changed the title Bug: jstrdecode(1) man page contains misleading text about -Q Bug: jstrdecode -Q is not enclosing the output in double quotes Sep 9, 2024
@lcn2
Copy link
Contributor Author

lcn2 commented Sep 9, 2024

See also GH-issuecomment-2337677465.

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 9, 2024

proposed new option for jstrdecode

In issue #11, we proposed a new -m option for jstrencode(1) (to ignore enclosing double-quotes around each string arg) as well as a clarification for how -Q (ignore enclosing double-quotes around the concatenation of all string args).

We propose a similar thing for jstrdecode(1).

The -Q would add double quotes around the concatenation of the decode of all args.

The -m would add backslashed escaped double quotes around each arg.

For example:

$ jstrdecode one two three
onetwothree
$ jstrdecode -Q one two three
"onetwothree"
$ jstrdecode -m one two three
\"one\"\"two\"\"three\"
$ jstrdecode -Q -m one two three
"\"one\"\"two\"\"three\""

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

Well the -Q is easy to do without much thought. Maybe the other parts are too but I am unfortunately not clear headed enough right now to read in detail.

I do want to know something though: these three issues you opened, where do these stand in priority wrt to the IOCCC28 ?

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

The initial bug also seems easy to fix. But I have other things to do now unfortunately, so...

QUESTION - priority wrt the IOCCC

As I have other things I have to do including over at mkiocccentry but also things unrelated to programming, I just wanted to ask what the priority of this issue is versus the IOCCC.

As a programmer, and I'm sure you are very familiar with this, this is an issue that I do not like having, but I also know there is such a thing as prioritising so I am asking this here. I hope to look at the is_available.sh script sometime soon but I have those other things I have to do too so I'm not sure what I'll get done today.

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

I've fixed -Q and I now know why not -q: quiet mode.

The other one should be easy to add too. I will see if I can do this quickly and then must do other things.

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

How do you propose the test suite check that -Q is done right though?

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

Pretty sure I have both but I changed the option to the more natural -e. We can talk about that later. I have to check output and if all good update the man page. When that is done I can commit and if it looks good to you we can close this as complete.

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

Actually I thought I had it but there's a problem that has to be worked out with stdin. The problem is that when do we write the "? It cannot be before we decode the string as that routine reads from stdin in that case and it would result in a " being added to the output prior to the user inputting something.

But with the -Q option we have to write a quote before and after the entire output. Now since the option arg can be - for stdin this presents a problem.

I will have to think about this. I'm afraid I have run out of time. If it was not for this I would be able to commit it now.

If you have thoughts on this feel free to add them but I'm sure I can work it out when I have more time. After my zoom call (at the hour, 45 minutes long) I have to do other things. I might have more time later but I'm not sure what I'll get done.

I will likely be able to do more later but we'll see.

@xexyl
Copy link
Owner

xexyl commented Sep 9, 2024

I think I have a solution. Stay tuned! (though it might be some time before I do it)

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 10, 2024

Pretty sure I have both but I changed the option to the more natural -e. We can talk about that later. I have to check output and if all good update the man page. When that is done I can commit and if it looks good to you we can close this as complete.

We have no strong opinion other than the option letter should not conflict with existing options.

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 10, 2024

The initial bug also seems easy to fix. But I have other things to do now unfortunately, so...

QUESTION - priority wrt the IOCCC

As I have other things I have to do including over at mkiocccentry but also things unrelated to programming, I just wanted to ask what the priority of this issue is versus the IOCCC.

As a programmer, and I'm sure you are very familiar with this, this is an issue that I do not like having, but I also know there is such a thing as prioritising so I am asking this here. I hope to look at the is_available.sh script sometime soon but I have those other things I have to do too so I'm not sure what I'll get done today.

The bug potentially impacts the formation of we pages as we now need to either decode \u<hex><hex><hex><hex> characters found in JSON, or force one to change non-ASCII characters in JSON into ASCII quasi-equivalents.

FYI: See GH-issuecomment-2339392055 for a remark on priories.

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 10, 2024

How do you propose the test suite check that -Q is done right though?

A shell script tight just does a few tests such as to test if:

jstedecode -Q -n 'l\u00e1maty\u00e1v\u00eb'

exists 0, and produces "lámatyávë".

A just few simple tests such as that should suffice.

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 10, 2024

We believe we have addressed all of the current questions that still need answering at this time. If we've missed something or something else needs to be clarified, please ask again.

@xexyl
Copy link
Owner

xexyl commented Sep 13, 2024

I believe that as of yesterday's commit (also done in mkiocccentry) this issue is fixed except that the jstr_test.sh script does not check for the output. I should have put the comments in the other thread here. Sorry about that!

Anyway, besides the tests, does this issue seem good?

Of course the decoding/encoding issue is another matter entirely but that is in the other issue, right?

@xexyl
Copy link
Owner

xexyl commented Sep 13, 2024

I believe with commit d5eaafa this issue can be marked closed as complete. Is this correct?

The jstr_test.sh now checks that -Q and -e work both by themselves and also together and all is good.

The decoding problems can be worked out in their respective issues, I believe?

I will try and sync this to mkiocccentry soon but I do really have other things I must get done.

UPDATE 0

The mkiocccentry repo has had this synced now too. Off to take care of other things. Assuming that this issue can be marked complete I will look at the other ones, hopefully starting tomorrow.

@lcn2
Copy link
Contributor Author

lcn2 commented Sep 14, 2024

Thank you and we recommend jparse repo PR 15 as a follow-up.

@lcn2 lcn2 closed this as completed Sep 14, 2024
@xexyl
Copy link
Owner

xexyl commented Sep 14, 2024

Thank you and we recommend jparse repo PR 15 as a follow-up.

You're welcome .. merged that a few minutes ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants