Clojure

star-directive in clojure.pprint/cl-format with an at-prefix ("~n@*") do not obey its specifications

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.4, Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

The star-directive in clojure.pprint/cl-format with an at-prefix (~n@*) does not obey its specifications according to Common Lisp the Language, 2nd Edition. There are two bugs within ~n@* as of right now:

  1. When ~n@* is supposed to jump forward over more than one argument, it jumps one step backward as if it had seen ~:*. For instance, (cl-format nil "~D ~3@*~D" 0 1 2 3) will return "0 0" and not "0 3" as expected.
  2. When ~@* is seen, the formatter is supposed to jump to the first argument (as n defaults to 0, see specification linked above). However, whenever a ~@*-directive is seen, the formatter jumps to the second argument instead.

Inside a clean Clojure repl, perform these steps:

user=> (require '[clojure.pprint :refer [cl-format]])
nil
user=> (cl-format nil "~D ~3@*~D" 0 1 2 3)
"0 0"                                           ;; Expected: "0 3"
user=> (cl-format nil "~D~D~D~D ~@*~D" 0 1 2 3)
"0123 1"                                        ;; Expected: "0123 0"

The format strings which reproduce the problem has been compared with the format function from the Common Lisp implementations SBCL, CLisp and Clozure. All of them print the expected output.

Patch: clj-1134-star-directive-in-cl-format.txt

Screened by: Alex Miller

Activity

Hide
Jean Niklas L'orange added a comment - - edited

Patch attached.

It may be easier to read the changes the patch does from within JIRA instead from the commit message, so I've added it here:

This solves two issues as specified by #CLJ-1134. Issue #1 is solved by doing a
relative jump forward within absolute-reposition in cl_format.clj, line 114 by
switching (- (:pos navigator) position) with (- position (:pos navigator)).

Issue #2 is handled by changing the default n-parameter to * depending on
whether the @-prefix is placed or not. If it is placed, then n defaults to
0, otherwise it defaults to 1.

In addition, new tests have been appended to test_cl_format.clj to ensure the
correctness of this patch. The tests have been tested on the Common Lisp
implementation GNU CLISP 2.49, which presumably handle the ~n@*
correctly. This patch and GNU CLISP returns the same output for each format
call, sans case for printed symbols; Common Lisp has case-insensitive symbols,
whereas Clojure has not.

Show
Jean Niklas L'orange added a comment - - edited Patch attached. It may be easier to read the changes the patch does from within JIRA instead from the commit message, so I've added it here: This solves two issues as specified by #CLJ-1134. Issue #1 is solved by doing a relative jump forward within absolute-reposition in cl_format.clj, line 114 by switching (- (:pos navigator) position) with (- position (:pos navigator)). Issue #2 is handled by changing the default n-parameter to * depending on whether the @-prefix is placed or not. If it is placed, then n defaults to 0, otherwise it defaults to 1. In addition, new tests have been appended to test_cl_format.clj to ensure the correctness of this patch. The tests have been tested on the Common Lisp implementation GNU CLISP 2.49, which presumably handle the ~n@* correctly. This patch and GNU CLISP returns the same output for each format call, sans case for printed symbols; Common Lisp has case-insensitive symbols, whereas Clojure has not.
Hide
Tom Faulhaber added a comment -

I walked through this patch and it looks just right. Thanks!

Let's get it applied for 1.7.

Show
Tom Faulhaber added a comment - I walked through this patch and it looks just right. Thanks! Let's get it applied for 1.7.
Hide
Jean Niklas L'orange added a comment -

I'm assuming this will not get in 1.7 as the RC2 is out right now, but I wish it could be prioritised into 1.8.

As it is a triaged bugfix that applies cleanly, I'm not sure there's anything more I can do here. But if there is, don't hesitate to ask for it.

Show
Jean Niklas L'orange added a comment - I'm assuming this will not get in 1.7 as the RC2 is out right now, but I wish it could be prioritised into 1.8. As it is a triaged bugfix that applies cleanly, I'm not sure there's anything more I can do here. But if there is, don't hesitate to ask for it.
Hide
Alex Miller added a comment -

Pre-screened for next release.

Show
Alex Miller added a comment - Pre-screened for next release.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated: