[CLJ-1078] Added queue, queue* and queue? to clojure.core Created: 26/Sep/12 Updated: 31/May/13 |
|
| Status: | Open |
| Project: | Clojure |
| Component/s: | None |
| Affects Version/s: | Release 1.5 |
| Fix Version/s: | None |
| Type: | Enhancement | Priority: | Trivial |
| Reporter: | Timothy Baldridge | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 3 |
| Labels: | data-structures, queue | ||
| Attachments: |
|
| Patch: | Code and Test |
| Description |
|
Original patch added functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests. In the updated patch, "queue" accepts a single collection argument as per Rich's suggestion, and does not provide a "queue*". |
| Comments |
| Comment by Andy Fingerhut [ 28/Sep/12 8:43 AM ] |
|
Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both. |
| Comment by Timothy Baldridge [ 26/Oct/12 5:23 PM ] |
|
Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go. |
| Comment by Andy Fingerhut [ 26/Oct/12 5:43 PM ] |
|
Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading). % git clone git://github.com/clojure/clojure.git |
| Comment by Timothy Baldridge [ 26/Oct/12 6:08 PM ] |
|
I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above. |
| Comment by Andy Fingerhut [ 26/Oct/12 6:26 PM ] |
|
Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method. |
| Comment by Timothy Baldridge [ 26/Oct/12 9:15 PM ] |
|
Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method. |
| Comment by Timothy Baldridge [ 26/Oct/12 9:16 PM ] |
|
added patch |
| Comment by Andy Fingerhut [ 26/Oct/12 9:37 PM ] |
|
That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks. |
| Comment by Rich Hickey [ 29/Nov/12 9:54 AM ] |
|
we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front. |
| Comment by Andy Fingerhut [ 11/Dec/12 1:00 PM ] |
|
Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent |
| Comment by Steve Miner [ 06/Apr/13 8:06 AM ] |
|
See also CLJ-976 (tagged literal support for PersistentQueue) |
| Comment by John Jacobsen [ 23/May/13 8:54 PM ] |
|
Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now. |
| Comment by John Jacobsen [ 26/May/13 9:04 AM ] |
|
Discussion initiated on clojure-dev: https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/2BOqHm24Vc4 |
| Comment by John Jacobsen [ 31/May/13 9:58 AM ] |
|
This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion. |
[CLJ-976] Implement reader literal and print support for PersistentQueue data structure Created: 27/Apr/12 Updated: 06/Apr/13 |
|
| Status: | Open |
| Project: | Clojure |
| Component/s: | None |
| Affects Version/s: | Release 1.5 |
| Fix Version/s: | None |
| Type: | Enhancement | Priority: | Minor |
| Reporter: | Fogus | Assignee: | Fogus |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | data-structures, queue, reader, tagged-literals | ||
| Attachments: |
|
| Patch: | Code and Test |
| Approval: | Test |
| Description |
|
Clojure's PersistentQueue structure has been in the language for quite some time now and has found its way into a fair share of codebases. However, the creation of queues is a two step operation often of the form: (conj clojure.lang.PersistentQueue/EMPTY :a :b :c) ;=> #<PersistentQueue clojure.lang.PersistentQueue@78d5f6bc> A better experience might be the following: #queue [:a :b :c] ;=> #queue [:a :b :c] (pop #queue [:a :b :c]) ;=> #queue [:b :c] This syntax is proposed and discussed in the Clojure-dev group at https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/GQqus5Wycno Open question: Should the queue literal's arguments eval? The implications of this are illustrated below: ;; non-eval case #queue [1 2 (+ 1 2)] ;=> #queue [1 2 (+ 1 2)] ;; eval case #queue [1 2 (+ 1 2)] ;=> #queue [1 2 3] The answer to this open question will determine the implementation. |
| Comments |
| Comment by Steve Miner [ 27/Apr/12 10:18 AM ] |
|
I think the non-eval behavior would be consistent with the other reader literals in Clojure 1.4. It's definitely better for interop where some other language implementation could be expected to handle a few literal representations, but not the evaluation of Clojure expressions. Use a regular function if the args need evaluation. |
| Comment by Chas Emerick [ 27/Apr/12 10:19 AM ] |
|
The precedent of records seems relevant: => (defrecord A [b])
user.A
=> #user.A[(+ 4 5)]
#user.A{:b (+ 4 5)}
=> #user.A{:b (+ 4 5)}
#user.A{:b (+ 4 5)}
This continues to make sense, as otherwise queues would need to print with an extra (quote …) form around lists — which records neatly avoid: => (A. '(+ 4 5))
#user.A{:b (+ 4 5)}
Does this mean that a queue fn (analogous to vector, maybe) will also make an appearance? It'd be handy for HOF usage. |
| Comment by Fogus [ 27/Apr/12 11:00 AM ] |
|
Added a patch for the tagged literal support ONLY. This is only one part of the total solution. This provides the read-string and printing capability. I'd like more discussion around the eval side before I get dive into the compiler. |
| Comment by Paul Michael Bauer [ 27/Apr/12 6:45 PM ] |
|
In addition to Chas' observations on consistency with record literals, would eval in queue literals open up the same security hole as #=, needing to respect *read-eval*? |
| Comment by Fogus [ 04/May/12 1:14 PM ] |
|
Evalable queue literal support. |
| Comment by Andy Fingerhut [ 10/May/12 5:54 PM ] |
|
Neither of the patches CLJ-976-queue-literal-tagged-parse-support-only.diff dated Apr 27, 2012 nor CLJ-976-queue-literal-eval.diff dated May 4, 2012 apply cleanly to latest master as of May 10, 2012. |
| Comment by Fogus [ 11/May/12 10:15 AM ] |
|
Updated patch file to merge with latest master. |
| Comment by Fogus [ 20/Jul/12 1:14 PM ] |
|
New patch with support fixed for syntax-quote. |
| Comment by Stuart Sierra [ 17/Aug/12 12:41 PM ] |
|
Patch does not apply as of commit f5f4faf95051f794c9bfa0315e4457b600c84cef |
| Comment by Fogus [ 17/Aug/12 3:06 PM ] |
|
Weird. I was able to download the CLJ-976-queue-literal-eval-and-synquote.diff patch and apply it to HEAD as of just now (f5f4faf95051f794c9bfa0315e4457b600c84cef). There were whitespace warnings, but the patch applied, compiles and passes all tests. |
| Comment by Andy Fingerhut [ 17/Aug/12 7:29 PM ] |
|
With latest head I was able to successfully apply patch CLJ-976-queue-literal-eval-and-synquote.diff with this command: git am --keep-cr -s < CLJ-976-queue-literal-eval-and-synquote.diff with some warnings, but successfully applied. If I try it without the --keep-cr option, the patch fails to apply. I believe this is often a sign that either one of the files being patched, or the patch itself, contains CR/LF line endings, which some of the Clojure source files definitely do. The command above (with --keep-cr) is currently the one recommended for applying patches on page http://dev.clojure.org/display/design/JIRA+workflow in section "Screening Tickets". I added the suggested --keep-cr option after running across another patch that applied with the option, but not without it. |
| Comment by Andy Fingerhut [ 28/Aug/12 5:45 PM ] |
|
Presumptuously changing Approval from Incomplete back to Test, since the latest patch does apply cleanly if --keep-cr option is used. |
| Comment by Rich Hickey [ 08/Sep/12 6:48 AM ] |
|
this needs more time |
| Comment by Fogus [ 18/Sep/12 8:15 AM ] |
|
Rich, Do you mind providing a little more detail? I would be happy to make any changes if needed. However, if it's just a matter of its relationship to EDN and/or waiting until the next release then I am happy to wait. In either case, I'd like to complete this or push it to the back of my mind. Thanks. |
| Comment by Andy Fingerhut [ 05/Oct/12 7:49 AM ] |
|
clj-976-queue-literal-eval-and-synquote-patch-v2.txt dated Oct 5 2012 is identical to Fogus's patch CLJ-976-queue-literal-eval-and-synquote.diff dated Jul 20 2012. It simply removes one line addition to clojure.iml that Rich has since added in a different commit, so that this patch now applies cleanly to latest master. |
| Comment by Andy Fingerhut [ 16/Oct/12 12:20 PM ] |
|
clj-976-queue-literal-eval-and-synquote-patch-v3.txt dated oct 16 2012 is identical to Fogus's patch CLJ-976-queue-literal-eval-and-synquote.diff dated Jul 20 2012. It simply removes one line addition to clojure.iml that Rich has since added in a different commit, so that this patch now applies cleanly to latest master. |
| Comment by Andy Fingerhut [ 20/Oct/12 12:26 PM ] |
|
Fogus, with the recent commit of a patch for |
| Comment by Steve Miner [ 06/Apr/13 8:07 AM ] |
|
Related to CLJ-1078. |