>>> 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 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 > 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 >> >> >