unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
@ 2015-10-08 15:07 Kaushal Modi
  2015-10-08 16:08 ` Jay Belanger
  0 siblings, 1 reply; 11+ messages in thread
From: Kaushal Modi @ 2015-10-08 15:07 UTC (permalink / raw)
  To: 21648, jay.p.belanger

[-- Attachment #1: Type: text/plain, Size: 3768 bytes --]

Hi,

I read this question of emacs.stackexchange (
http://emacs.stackexchange.com/q/13451/115 ) where the user needed to
specify the radix of the number he was pasting in calc.

If the calc default radix is decimal and if a user pastes 1000, it will be
pasted as decimal 1000. But what if the user meant to paste binary 1000
(decimal 8)?

My patch below enables doing that using numeric prefixes.

Please advise if merging this patch to calc-yank is a good idea or if needs
improvement/bug fixes before the merging. I have used this modified
calc-yank function for few weeks and did not mess up anything else in my
calc usage.




======================================================

From fa9c8f6c2cce1c9e69bd6f6c25f036a72e0b799b Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Thu, 8 Oct 2015 10:56:36 -0400
Subject: [PATCH] Prepend radix to yanked numbers using num prefix

If 1000 is the last element saved to the kill-ring.

Then,

C-2 C-y will paste 8 (2#1000),
C-8 C-y will paste 512 (8#1000),
C-0 C-y will paste 1000 (10#1000),
C-6 C-y will paste 4096 (16#1000)
.. and C-y will paste 1000 (1000).
---
 lisp/calc/calc-yank.el | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/lisp/calc/calc-yank.el b/lisp/calc/calc-yank.el
index 5694a4e..811b308 100644
--- a/lisp/calc/calc-yank.el
+++ b/lisp/calc/calc-yank.el
@@ -111,25 +111,40 @@ calc-copy-region-as-kill
 ;; otherwise it just parses the yanked string.
 ;; Modified to use Emacs 19 extended concept of kill-ring. -- daveg
12/15/96
 ;;;###autoload
-(defun calc-yank ()
-  (interactive)
+(defun calc-yank (radix)
+  "Yank a value into the Calculator buffer.
+
+If RADIX is nil, do not prepend any radix notation.
+
+If RADIX is 2, prepend \"2#\" (The yanked number will be perceived as
binary.)
+If RADIX is 8, prepend \"8#\" (The yanked number will be perceived as
octal.)
+If RADIX is 0, prepend \"10#\" (The yanked number will be perceived as
decimal.)
+If RADIX is 16, prepend \"16#\" (The yanked number will be perceived as
hexadecimal.) "
+  (interactive "P")
   (calc-wrapper
    (calc-pop-push-record-list
     0 "yank"
-    (let ((thing (if (fboundp 'current-kill)
-     (current-kill 0 t)
-   (car kill-ring-yank-pointer))))
+    (let* ((radix-notation (cl-case radix
+                             (2 "2#")
+                             (8 "8#")
+                             (0 "10#")
+                             (6 "16#")
+                             (t "")))
+           (thing (concat radix-notation
+                          (if (fboundp 'current-kill)
+                              (current-kill 0 t)
+                            (car kill-ring-yank-pointer)))))
       (if (eq (car-safe calc-last-kill) thing)
-  (cdr calc-last-kill)
- (if (stringp thing)
-    (let ((val (math-read-exprs (calc-clean-newlines thing))))
-      (if (eq (car-safe val) 'error)
-  (progn
-    (setq val (math-read-exprs thing))
-    (if (eq (car-safe val) 'error)
- (error "Bad format in yanked data")
-      val))
- val))))))))
+          (cdr calc-last-kill)
+        (if (stringp thing)
+            (let ((val (math-read-exprs (calc-clean-newlines thing))))
+              (if (eq (car-safe val) 'error)
+                  (progn
+                    (setq val (math-read-exprs thing))
+                    (if (eq (car-safe val) 'error)
+                        (error "Bad format in yanked data")
+                      val))
+                val))))))))

 ;;; The Calc set- and get-register commands are modified versions of
functions
 ;;; in register.el
-- 
2.6.0.rc0.24.gec371ff

======================================================


PS: My copyright paperwork is on file (#1029578)

--
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 13831 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 15:07 bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank Kaushal Modi
@ 2015-10-08 16:08 ` Jay Belanger
  2015-10-08 16:19   ` Kaushal Modi
  2015-10-08 16:30   ` Eli Zaretskii
  0 siblings, 2 replies; 11+ messages in thread
From: Jay Belanger @ 2015-10-08 16:08 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 21648


Hi Kaushal,

> I read this question of emacs.stackexchange
> ( http://emacs.stackexchange.com/q/13451/115)
> where the user needed to specify the radix of the number he was
> pasting in calc. 
>
> If the calc default radix is decimal and if a user pastes 1000, it
> will be pasted as decimal 1000. But what if the user meant to paste
> binary 1000 (decimal 8)?
>
> My patch below enables doing that using numeric prefixes.
>
> Please advise if merging this patch to calc-yank is a good idea or if
> needs improvement/bug fixes before the merging.

There's a feature freeze on right now, so it shouldn't be added to Emacs
right away.  But it looks useful.

With the patch, if the yanked number already has the radix prefix, there
is an error.  It might make more sense to have Calc do an appropriate
conversion. Also, the number of radixes in the patch is less than Calc allows.
It might make more sense to have calc-yank use the current Calc's
current radix rather than a prefix radix.
I don't recall the policy on using cl- functions, but cond could
easily be used instead of cl-case.

But this should be brought up again after the feature freeze.

Jay






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 16:08 ` Jay Belanger
@ 2015-10-08 16:19   ` Kaushal Modi
  2015-10-08 16:59     ` Kaushal Modi
  2015-10-08 16:30   ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Kaushal Modi @ 2015-10-08 16:19 UTC (permalink / raw)
  To: jay.p.belanger; +Cc: 21648

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

Thanks for the review!

I will work out these kinks by the time we can add in more features.
I'll update this thread then.


--
Kaushal Modi

On Thu, Oct 8, 2015 at 12:08 PM, Jay Belanger <jay.p.belanger@gmail.com>
wrote:

>
> Hi Kaushal,
>
> > I read this question of emacs.stackexchange
> > ( http://emacs.stackexchange.com/q/13451/115)
> > where the user needed to specify the radix of the number he was
> > pasting in calc.
> >
> > If the calc default radix is decimal and if a user pastes 1000, it
> > will be pasted as decimal 1000. But what if the user meant to paste
> > binary 1000 (decimal 8)?
> >
> > My patch below enables doing that using numeric prefixes.
> >
> > Please advise if merging this patch to calc-yank is a good idea or if
> > needs improvement/bug fixes before the merging.
>
> There's a feature freeze on right now, so it shouldn't be added to Emacs
> right away.  But it looks useful.
>
> With the patch, if the yanked number already has the radix prefix, there
> is an error.  It might make more sense to have Calc do an appropriate
> conversion. Also, the number of radixes in the patch is less than Calc
> allows.
> It might make more sense to have calc-yank use the current Calc's
> current radix rather than a prefix radix.
> I don't recall the policy on using cl- functions, but cond could
> easily be used instead of cl-case.
>
> But this should be brought up again after the feature freeze.
>
> Jay
>
>

[-- Attachment #2: Type: text/html, Size: 2563 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 16:08 ` Jay Belanger
  2015-10-08 16:19   ` Kaushal Modi
@ 2015-10-08 16:30   ` Eli Zaretskii
  2015-10-08 18:11     ` Jay Belanger
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2015-10-08 16:30 UTC (permalink / raw)
  To: jay.p.belanger; +Cc: 21648, kaushal.modi

> From: Jay Belanger <jay.p.belanger@gmail.com>
> Date: Thu, 08 Oct 2015 11:08:33 -0500
> Cc: 21648@debbugs.gnu.org
> 
> There's a feature freeze on right now

No, there was no decision yet to start the feature freeze.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 16:19   ` Kaushal Modi
@ 2015-10-08 16:59     ` Kaushal Modi
  2015-10-08 19:26       ` Jay Belanger
  0 siblings, 1 reply; 11+ messages in thread
From: Kaushal Modi @ 2015-10-08 16:59 UTC (permalink / raw)
  To: Jay Belanger, 21648

[-- Attachment #1: Type: text/plain, Size: 3755 bytes --]

>>> Eli Zaretskii
> No, there was no decision yet to start the feature freeze.

In that case, here is my feedback on a quick review.

>>> Jay Belanger
> With the patch, if the yanked number already has the radix prefix, there
> is an error.  It might make more sense to have Calc do an appropriate
> conversion.
I did not try that case as I meant to use the prefix only when the copied
number does not have a calc-style prefix. But that's a valid point. If the
copied number has calc-style prefix, what should have a higher priority?
- The radix translated by the copied number with prefix, or
- The radix conveyed by the user specified prefix to calc-yank?
I believe that if the user has intentionally used the prefix, the calc-yank
prefix should override. Either way, it will be easy with some string
manipulation.

> Also, the number of radixes in the patch is less than Calc allows.
I assumed the use cases of only the common radixes used in programming. How
about I display a prompt for the user to enter any radix they like (I
believe calc supports up to radix 36) if the prefix is specifically '(4) (
a list prefix created when user uses C-u specifically for prefix).

So now, C-2 C-y will prefix the yanked number with "2#". But C-u C-y will
show a prompt where the user will enter radix. If the user entered 9, the
prefix appended will be "9#". Does this option sound fair?

> It might make more sense to have calc-yank use the current Calc's current
radix rather than a prefix radix.
I need some clarification for this point. Did you mean that if the user has
set the calc radix to hex by using "d6" and now if they yank a number
"1000" it gets yanked automatically as "16#1000". If yes, I believe it will
cause a huge disturbance in the way people might have already got used to
yanking in calc.

> I don't recall the policy on using cl- functions, but cond could easily
be used instead of cl-case.
I personally find cl-case syntax very concise and so I used that. In a
thread in emacs-devel, there is/was a discussion on if cl-lib should be
preloaded automatically. I believe that until that decision is reached, I
should use cond instead of cl-case.


--
Kaushal Modi

On Thu, Oct 8, 2015 at 12:19 PM, Kaushal Modi <kaushal.modi@gmail.com>
wrote:

> Thanks for the review!
>
> I will work out these kinks by the time we can add in more features.
> I'll update this thread then.
>
>
> --
> Kaushal Modi
>
> On Thu, Oct 8, 2015 at 12:08 PM, Jay Belanger <jay.p.belanger@gmail.com>
> wrote:
>
>>
>> Hi Kaushal,
>>
>> > I read this question of emacs.stackexchange
>> > ( http://emacs.stackexchange.com/q/13451/115)
>> > where the user needed to specify the radix of the number he was
>> > pasting in calc.
>> >
>> > If the calc default radix is decimal and if a user pastes 1000, it
>> > will be pasted as decimal 1000. But what if the user meant to paste
>> > binary 1000 (decimal 8)?
>> >
>> > My patch below enables doing that using numeric prefixes.
>> >
>> > Please advise if merging this patch to calc-yank is a good idea or if
>> > needs improvement/bug fixes before the merging.
>>
>> There's a feature freeze on right now, so it shouldn't be added to Emacs
>> right away.  But it looks useful.
>>
>> With the patch, if the yanked number already has the radix prefix, there
>> is an error.  It might make more sense to have Calc do an appropriate
>> conversion. Also, the number of radixes in the patch is less than Calc
>> allows.
>> It might make more sense to have calc-yank use the current Calc's
>> current radix rather than a prefix radix.
>> I don't recall the policy on using cl- functions, but cond could
>> easily be used instead of cl-case.
>>
>> But this should be brought up again after the feature freeze.
>>
>> Jay
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 7972 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 16:30   ` Eli Zaretskii
@ 2015-10-08 18:11     ` Jay Belanger
  0 siblings, 0 replies; 11+ messages in thread
From: Jay Belanger @ 2015-10-08 18:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21648, kaushal.modi


>> There's a feature freeze on right now
>
> No, there was no decision yet to start the feature freeze.

Sorry then; I misremembered.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 16:59     ` Kaushal Modi
@ 2015-10-08 19:26       ` Jay Belanger
  2015-10-08 20:35         ` Kaushal Modi
  0 siblings, 1 reply; 11+ messages in thread
From: Jay Belanger @ 2015-10-08 19:26 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 21648


>> With the patch, if the yanked number already has the radix prefix, there
>> is an error. It might make more sense to have Calc do an appropriate
>> conversion. 
> I did not try that case as I meant to use the prefix only when the copied number does
> not have a calc-style prefix. But that's a valid point. If the copied number has
> calc-style prefix, what should have a higher priority?

If the copied number has a calc-style prefix, then the number should be
interpreted in that base.  I can be entered onto the stack in the new
base.  That's done now; if Calc is using base 5 and 2#111 is yanked,
then 5#12 appears on the stack.

> Either way, it will be easy with some string manipulation.

You'd probably want Calc to do the conversion.

>> It might make more sense to have calc-yank use the current Calc's current radix rather
>> than a prefix radix.
> I need some clarification for this point. Did you mean that if the
> user has set the calc radix to hex by using "d6" and now if they yank
> a number "1000" it gets yanked automatically as "16#1000".

Yes; that's what I meant.

> If yes, I believe it will cause a huge disturbance in the way people
> might have already got used to yanking in calc.

You are probably right.  I would prefer the yanking to pay attention to
the current Calc setting, but I really don't like changing age-old
behavior.  Your method is a good approach.

>> Also, the number of radixes in the patch is less than Calc allows.
> I assumed the use cases of only the common radixes used in programming. How about I
> display a prompt for the user to enter any radix they like (I believe calc supports up
> to radix 36) if the prefix is specifically '(4) ( a list prefix created when user uses
> C-u specifically for prefix).
>
> So now, C-2 C-y will prefix the yanked number with "2#". But C-u C-y will show a prompt
> where the user will enter radix. If the user entered 9, the prefix appended will be
> "9#". Does this option sound fair?

Yes; it sounds like a nice approach.  And perhaps some sort of message
if the entered radix isn't a number from 2 to 36 (the allowed radixes).

Thanks again,
Jay







^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 19:26       ` Jay Belanger
@ 2015-10-08 20:35         ` Kaushal Modi
  2015-10-09  1:04           ` Jay Belanger
  0 siblings, 1 reply; 11+ messages in thread
From: Kaushal Modi @ 2015-10-08 20:35 UTC (permalink / raw)
  To: Jay Belanger; +Cc: 21648

[-- Attachment #1: Type: text/plain, Size: 7643 bytes --]

Hi all,

Here the the new implementation of calc-yank. I can submit a PATCH once the
new definition is confirmed.

> If the copied number has a calc-style prefix, then the number should be
> interpreted in that base.  I can be entered onto the stack in the new
> base.  That's done now; if Calc is using base 5 and 2#111 is yanked,
> then 5#12 appears on the stack.

It does that now
​. I also tested with the 2#111 and few other examples.

​

​> ​
You'd probably want Calc to do the conversion.

​Correct, I am not doing any string manipulation.. I simply don't handle
the string if it begins with "\\`[0-9]+#".​

​> ​
Your method is a good approach.

​Thanks. I have not changed the way ​it works right now. Using the same
example of yanking "1000" in a calc buffer with radix set to hex ("d6"),
doing C-y will paste 16#3E8. But with the new calc-yank, C-6 C-y will paste
16#1000.

​> ​
Yes; it sounds like a nice approach.

​Done! I have implemented C-u C-y functionality which will prompt user to
enter radix between 2 and 36.​

​> ​
And perhaps some sort of message
​> ​
if the entered radix isn't a number from 2 to 36 (the allowed radixes).

Is that check needed in calc-yank? If I enter an invalid radix like 37, I
get the generic error "Bad format in yanked data".​

> ​Also, you probably want to make sure that calc-yank behaves
​> ​
appropriately when a number isn't being yanked.
​> ​
(Either applying the new base to all numbers in an expression or
​> ​
disregarding the radix when the expression being yanked isn't a pure
> number, for example.)

Ah! I wasn't aware that yanking non-numbers was possible!
Based on the current code, as long as the user does not provide ANY prefix,
calc-yank will work exactly as before. So the non-number yanking should
also behave the same way.

Can you please provide feedback on how to incorporate that based on the
below updated code?
Or can you provide some valid non-number examples for yanking?



==============================================================​

(defun calc-yank (radix)
          "Yank a value into the Calculator buffer.

Valid numeric prefixes for RADIX: 0, 2, 6, 8
No radix notation is prepended for any other numeric prefix.

If RADIX is 2, prepend \"2#\"  - Binary.
If RADIX is 8, prepend \"8#\"  - Octal.
If RADIX is 0, prepend \"10#\" - Decimal.
If RADIX is 6, prepend \"16#\" - Hexadecimal.

If RADIX is a non-nil list (created using \\[universal-argument]), the user
will be prompted to enter the radix in the minibuffer.

If RADIX is nil or if the yanked string already has a calc radix prefix, the
yanked string will be passed on directly to the Calculator buffer without
any
alteration."
          (interactive "P")
          (calc-wrapper
           (calc-pop-push-record-list
            0 "yank"
            (let* (radix-notation
                   (thing-raw (if (fboundp 'current-kill)
                                  (current-kill 0 t)
                                (car kill-ring-yank-pointer)))
                   (thing (if (or (null radix)
                                  (string-match-p "\\`[0-9]+#" thing-raw))
                              thing-raw
                            (progn
                              (setq radix-notation
                                    (if (listp radix)
                                        (concat (number-to-string
                                                 (read-number
                                                  "Set radix for yanked
number (2-36): "))
                                                "#")
                                      (cond ((eq radix 2) "2#")
                                            ((eq radix 8) "8#")
                                            ((eq radix 0) "10#")
                                            ((eq radix 6) "16#")
                                            (t (progn
                                                 (message
                                                  (concat "No radix is "
                                                          "prepended for
prefix "
                                                          "value of %0d.
Valid "
                                                          "numeric prefixes
are "
                                                          "0, 2, 6, 8.")
                                                  radix)
                                                 "")))))
                              (concat radix-notation thing-raw)))))
              (if (eq (car-safe calc-last-kill) thing)
                  (cdr calc-last-kill)
                (if (stringp thing)
                    (let ((val (math-read-exprs (calc-clean-newlines
thing))))
                      (if (eq (car-safe val) 'error)
                          (progn
                            (setq val (math-read-exprs thing))
                            (if (eq (car-safe val) 'error)
                                (error "Bad format in yanked data")
                              val))
                        val))))))))


--
Kaushal Modi

On Thu, Oct 8, 2015 at 3:26 PM, Jay Belanger <jay.p.belanger@gmail.com>
wrote:
>
>
> >> With the patch, if the yanked number already has the radix prefix,
there
> >> is an error. It might make more sense to have Calc do an appropriate
> >> conversion.
> > I did not try that case as I meant to use the prefix only when the
copied number does
> > not have a calc-style prefix. But that's a valid point. If the copied
number has
> > calc-style prefix, what should have a higher priority?
>
> If the copied number has a calc-style prefix, then the number should be
> interpreted in that base.  I can be entered onto the stack in the new
> base.  That's done now; if Calc is using base 5 and 2#111 is yanked,
> then 5#12 appears on the stack.
>
> > Either way, it will be easy with some string manipulation.
>
> You'd probably want Calc to do the conversion.
>
> >> It might make more sense to have calc-yank use the current Calc's
current radix rather
> >> than a prefix radix.
> > I need some clarification for this point. Did you mean that if the
> > user has set the calc radix to hex by using "d6" and now if they yank
> > a number "1000" it gets yanked automatically as "16#1000".
>
> Yes; that's what I meant.
>
> > If yes, I believe it will cause a huge disturbance in the way people
> > might have already got used to yanking in calc.
>
> You are probably right.  I would prefer the yanking to pay attention to
> the current Calc setting, but I really don't like changing age-old
> behavior.  Your method is a good approach.
>
> >> Also, the number of radixes in the patch is less than Calc allows.
> > I assumed the use cases of only the common radixes used in programming.
How about I
> > display a prompt for the user to enter any radix they like (I believe
calc supports up
> > to radix 36) if the prefix is specifically '(4) ( a list prefix created
when user uses
> > C-u specifically for prefix).
> >
> > So now, C-2 C-y will prefix the yanked number with "2#". But C-u C-y
will show a prompt
> > where the user will enter radix. If the user entered 9, the prefix
appended will be
> > "9#". Does this option sound fair?
>
> Yes; it sounds like a nice approach.  And perhaps some sort of message
> if the entered radix isn't a number from 2 to 36 (the allowed radixes).
>
> Thanks again,
> Jay
>
>

[-- Attachment #2: Type: text/html, Size: 12596 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-08 20:35         ` Kaushal Modi
@ 2015-10-09  1:04           ` Jay Belanger
  2015-10-09  2:56             ` Kaushal Modi
  0 siblings, 1 reply; 11+ messages in thread
From: Jay Belanger @ 2015-10-09  1:04 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 21648


Hi!

> ​Done! I have implemented C-u C-y functionality which will prompt user to enter radix between 2 and 36.​
>
>> And perhaps some sort of message
>> if the entered radix isn't a number from 2 to 36 (the allowed radixes).
>
> Is that check needed in calc-yank? If I enter an invalid radix like
> 37, I get the generic error "Bad format in yanked data".​

But the data might be fine; the problem is with the radix.

>> ​Also, you probably want to make sure that calc-yank behaves
>> appropriately when a number isn't being yanked.
>
> Can you please provide feedback on how to incorporate that based on the below updated code?

To determine whether or not a string represents a number, you could have
Calc parse it, I suppose.  But that would be wasteful.
It might be better to add a function, maybe called
`math-numberstring-p', which returns t is the argument is a string
which represents a Calc number.  Then along with
  (or ((null radix)
      (string-match-p "\\`[0-9]+#" thing-raw)
      ...
you could add
      (not (math-numberstring-p thing-raw))

Oh, also, calc-yank can yank more than one line.  Currently, if

111
1111

is killed and yanked with C-8 C-y,
the result is

2:  73
1:  1111

and you'd probably want

2:  73
1:  585

Also, the message
  (message
  (concat "No radix is "
   "prepended for prefix "
  "value of %0d. Valid "
  "numeric prefixes are "
  "0, 2, 6, 8.")
   radix)
is a bit long.

> I can submit a PATCH once the new definition is confirmed.

I can commit this for you without a patch.

Jay





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
  2015-10-09  1:04           ` Jay Belanger
@ 2015-10-09  2:56             ` Kaushal Modi
       [not found]               ` <8737xk7nge.fsf@gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: Kaushal Modi @ 2015-10-09  2:56 UTC (permalink / raw)
  To: Jay Belanger; +Cc: 21648

[-- Attachment #1: Type: text/plain, Size: 4816 bytes --]

Thanks for the feedback.

The below code has every suggestion implemented except for the
`math-numberstring-p`.
I will need to thing more about how to implement that.

Also the below condition might not be enough (even before we start thinking
about math-numberstring-p):

(or (null radix)
     (string-match-p "\\`[0-9]+#" thing-raw))


What if the user yanks the below string?

111
2#1111

In that case, the below code will generate the below if called using C-2 C-y

2#111
2#2#1111

The above case seems impractical. Or is it? Should we need to make the code
foolproof against such unusual cases?
If yes, then we need to ensure that either the radix prepending happens
only if all yanked lines are pristine

The below code has the following updated:
- Support for negative numbers!
- Error when user enters a non-integer or an integer <2 or >36
- Support for prepending the radix-notation to multiline yanks (assuming
the yanks do not fit in the above mentioned strange case)
- Made the referenced long message a bit concise.

======

(defun calc-yank (radix)
          "Yank a value into the Calculator buffer.

Valid numeric prefixes for RADIX: 0, 2, 6, 8
No radix notation is prepended for any other numeric prefix.

If RADIX is 2, prepend \"2#\"  - Binary.
If RADIX is 8, prepend \"8#\"  - Octal.
If RADIX is 0, prepend \"10#\" - Decimal.
If RADIX is 6, prepend \"16#\" - Hexadecimal.

If RADIX is a non-nil list (created using \\[universal-argument]), the user
will be prompted to enter the radix in the minibuffer.

If RADIX is nil or if the yanked string already has a calc radix prefix, the
yanked string will be passed on directly to the Calculator buffer without
any
alteration."
          (interactive "P")
          (calc-wrapper
           (calc-pop-push-record-list
            0 "yank"
            (let* (radix-num
                   radix-notation
                   (thing-raw (if (fboundp 'current-kill)
                                  (current-kill 0 t)
                                (car kill-ring-yank-pointer)))
                   (thing (if (or (null radix)
                                  (string-match-p "\\`\\-*[0-9]+#"
thing-raw))
                              thing-raw
                            (progn
                              (setq radix-notation
                                    (if (listp radix)
                                        (progn
                                          (setq radix-num
                                                (read-number
                                                 "Set radix for yanked
number (2-36): "))
                                          (if (and (integerp radix-num)
                                                   (<= 2 radix-num)
                                                   (>= 36 radix-num))
                                              (concat (number-to-string
radix-num) "#")
                                            (error (concat "The radix has
to be an "
                                                           "integer between
2 and 36."))))
                                      (cond ((eq radix 2) "2#")
                                            ((eq radix 8) "8#")
                                            ((eq radix 0) "10#")
                                            ((eq radix 6) "16#")
                                            (t (progn
                                                 (message
                                                  (concat "No radix
prepended "
                                                          "for invalid
numeric "
                                                          "prefix %0d.")
                                                  radix)
                                                 "")))))
                              ;; Ensure that the radix-notation is prefixed
                              ;; correctly even for multi-line yanks like
                              ;; 111
                              ;; 1111
                              (replace-regexp-in-string
                               "^\\(\\-*\\)\\(.*\\)"
                               (concat "\\1" radix-notation "\\2")
                               thing-raw)))))
              (if (eq (car-safe calc-last-kill) thing)
                  (cdr calc-last-kill)
                (if (stringp thing)
                    (let ((val (math-read-exprs (calc-clean-newlines
thing))))
                      (if (eq (car-safe val) 'error)
                          (progn
                            (setq val (math-read-exprs thing))
                            (if (eq (car-safe val) 'error)
                                (error "Bad format in yanked data")
                              val))
                        val))))))))

--
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 14828 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank
       [not found]                                 ` <87vbaeib5o.fsf@gmail.com>
@ 2015-10-12 13:16                                   ` Kaushal Modi
  0 siblings, 0 replies; 11+ messages in thread
From: Kaushal Modi @ 2015-10-12 13:16 UTC (permalink / raw)
  To: jay.p.belanger, 21648-done

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

Closing this debbugs as the patch is now commited to the trunk:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ec0d4d24fd11b5040de9f7657b486c3b1e743071

Many thanks to Jay for a very active feedback session, helping make the
final patch much more robust than the initial version.

[-- Attachment #2: Type: text/html, Size: 443 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-10-12 13:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-08 15:07 bug#21648: 25.0.50; [PATCH] Add ability to specify radix for the yanked number in calc-yank Kaushal Modi
2015-10-08 16:08 ` Jay Belanger
2015-10-08 16:19   ` Kaushal Modi
2015-10-08 16:59     ` Kaushal Modi
2015-10-08 19:26       ` Jay Belanger
2015-10-08 20:35         ` Kaushal Modi
2015-10-09  1:04           ` Jay Belanger
2015-10-09  2:56             ` Kaushal Modi
     [not found]               ` <8737xk7nge.fsf@gmail.com>
     [not found]                 ` <CAFyQvY3YBu1x4wcib++K_mTGfRNLqBfPBaj90rYJriZAuk-9Gg@mail.gmail.com>
     [not found]                   ` <CAFyQvY04LqeGAwyWMGFFDpHDqkb1TbW+vfH9drHruq6+yhVRpw@mail.gmail.com>
     [not found]                     ` <87twq0dkqd.fsf@gmail.com>
     [not found]                       ` <CAFyQvY1-yu9QQhzaQLVv9MLLkBQmOxY=usidbguSJnr9xwKp6g@mail.gmail.com>
     [not found]                         ` <87vbael6ph.fsf@gmail.com>
     [not found]                           ` <CAFyQvY07v3P2bjUQ+PAJbvUQu_nXV=k+umVW-JeKK2r=uWS=_Q@mail.gmail.com>
     [not found]                             ` <87twpyuz1q.fsf@gmail.com>
     [not found]                               ` <CAFyQvY3BbgGimetANnnZ-P13KXvH54_QHqcYF3sAiPWFaz=NFA@mail.gmail.com>
     [not found]                                 ` <87vbaeib5o.fsf@gmail.com>
2015-10-12 13:16                                   ` Kaushal Modi
2015-10-08 16:30   ` Eli Zaretskii
2015-10-08 18:11     ` Jay Belanger

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).