sprintf error should be more awesome #759

Closed
Benabik opened this Issue Apr 26, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@Benabik
Member

Benabik commented Apr 26, 2012

Currently, sprintf str, "%;", [] throws the following exception: ';' is not a valid sprintf format. It would be more clear if it displayed the entire format there, including the %.

First thought on implementation: keep an index for the % character and then substr between there and the current position before throwing the exception.

@ghost ghost assigned Whiteknight May 9, 2012

@Whiteknight

This comment has been minimized.

Show comment
Hide comment
@Whiteknight

Whiteknight May 12, 2012

Contributor

I've started a new branch, whiteknight/sprintf_cleanup, to work on several issues related to the sprintf code, including this ticket. I will post updates as the work progresses.

Contributor

Whiteknight commented May 12, 2012

I've started a new branch, whiteknight/sprintf_cleanup, to work on several issues related to the sprintf code, including this ticket. I will post updates as the work progresses.

Whiteknight added a commit that referenced this issue May 20, 2012

A few small code cleanups. Fix t/op/sprintf.t, since it is wonderfull…
…y matching on the exact text of the exception message, which is changing for GH #759
@Whiteknight

This comment has been minimized.

Show comment
Hide comment
@Whiteknight

Whiteknight May 20, 2012

Contributor

@Benabik The error message has been updated to say:

"'%c' is not valid in sprintf format sequence '%Ss'"

Where the second parameter is the text of the format sequence so far. Once the state machine finds one incorrect character it can't keep going to find the complete intended format, so this is the best we can do. If this is acceptable to you, I'll merge my branch to master and close this ticket.

Contributor

Whiteknight commented May 20, 2012

@Benabik The error message has been updated to say:

"'%c' is not valid in sprintf format sequence '%Ss'"

Where the second parameter is the text of the format sequence so far. Once the state machine finds one incorrect character it can't keep going to find the complete intended format, so this is the best we can do. If this is acceptable to you, I'll merge my branch to master and close this ticket.

@Whiteknight

This comment has been minimized.

Show comment
Hide comment
@Whiteknight

Whiteknight May 20, 2012

Contributor

I have just merged this branch to master. The error message is improved as best as I can make it. I'm closing this ticket.

Contributor

Whiteknight commented May 20, 2012

I have just merged this branch to master. The error message is improved as best as I can make it. I'm closing this ticket.

@Whiteknight Whiteknight removed their assignment Mar 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment