Skip to content

Conversation

@furushchev
Copy link
Member

Closes #295

Test case is euslisp/jskeus#509

@Affonso-Gui
Copy link
Member

Having problems with :count in substitute too.

(substitute 'b 'a #(a b a d) :count 1)
;; #(b)

Will send a PR shortly.

@Affonso-Gui
Copy link
Member

Affonso-Gui commented Jul 26, 2018

PR in furushchev#1

@furushchev
Copy link
Member Author

furushchev commented Jul 26, 2018

@Affonso-Gui Thanks! Now both your PR to here and the test code were merged. 👍

@furushchev furushchev changed the title fix: remove/delete sequence with :end :count fix: remove/delete/substitute sequence with :end :count Jul 26, 2018
@Affonso-Gui
Copy link
Member

@k-okada ping

@k-okada
Copy link
Member

k-okada commented Aug 9, 2018

can we re-target this to cl-compatible branch?

@Affonso-Gui
Copy link
Member

Although :count problem is not a cl-compatibility issue, but rather a eus bug

irteusgl$ (remove 1 (vector 1 2 3 2 1) :count 1)  ;; -> #()

:end key is a bit more tricky, and may indeed affect existing programs.

irteusgl$ (delete 2 (vector 1 2 3 2 1) :end 2)     ;; -> #(1)

Considering so, it might indeed be a good idea to target cl-compatible.
@furushchev

@furushchev
Copy link
Member Author

furushchev commented Aug 9, 2018

@Affonso-Gui Exactly this is a bug rather than cl-compatibility issue but I don't think this change does not change the existing behavior so to affect existing programs because this PR is just for making :start, :end and :count work.
(The result of the two commands is not changed before/after this PR).

After this PR, the behavior is exactly the same as expected in the document.

remove item seq &key :start :end :test :test-not :key :count [関数]
seqの中の:start番目から:end番目までの要素のなかで、 itemと同一の要素を探し、:count (デフォルト値は∞)番目までの要素を削除した新しい列を作る。 もし、itemが一回のみ現れることが確定しているなら、 無意味な探索を避けるために、:count=1を指定すること。

delete item seq &key :start :end :test :test-not :key :count [関数]
deleteは、seq自体を修正し、新しい列を作らないことを除いては、 remove同じである。 もし、itemが一回のみ現れることが確定しているなら、 無意味な探索を避けるために、:count=1を指定すること。

(BTW, seq自体を修正し、新しい列を作らない this rule is only for euslisp. Other common lisp implementations does not follow the rule but just seqは破壊されるが、返り値はremoveと同じである. Please see https://github.com/euslisp/jskeus/pull/509/files#diff-3ca9928ee6707c14b0d977dcdca83e91R148 for detail.)

@k-okada
For the target branch of this PR, both are ok for me. 👍

@k-okada
Copy link
Member

k-okada commented Aug 9, 2018

yes but someone is already using this, let's merge into cl-compatible and also please use euslisp with this patch for your research demo and see if does not have any side effects

k-okada@p51s:~/catkin_ws/ws_euslib/src/euslib$ find -iname *.l -exec grep \(delete\  {} \; | grep :count
          (setq *process-pool* (delete target-process *process-pool* :count 1))
		   (delete tmp answer :count 1))
	(setq selected (delete (cadr selected) selected :count 1))
	(setq selected (delete (cadr selected) selected :count 1))
		(setq *vertices* (delete ip *vertices* :count 1)) )))
      (setq cand-verts (delete avert cand-verts :key #'car :count 1)))
			  (setq *vertices* (delete ver *vertices* :count 1)))
		 (setq newregion (delete (car p1) newregion :count 1))
		 (setq newregion (delete (car p2) newregion :count 1))
	       (setq regions (append newregion (delete aregion regions :count 1)))
  		     (setq *vertices* (delete cpv *vertices* :count 1))))
			   (setq *vertices* (delete ver *vertices* :count 1)))
      (setq open-list (delete min-x open-list :count 1))
	   (delete v uni :count 1))
	   (delete v uni :count 1))
	   (delete v uni :count 1))
	   (delete v uni :count 1))

@furushchev furushchev changed the base branch from master to cl-compatible August 9, 2018 04:55
@furushchev
Copy link
Member Author

@Affonso-Gui @k-okada Changed the target branch.

@k-okada k-okada merged commit 1904108 into euslisp:cl-compatible Aug 9, 2018
@furushchev furushchev deleted the remove branch August 11, 2018 01:12
@furushchev furushchev restored the remove branch September 11, 2018 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants