Op 11-sep-2007, om 23:59 heeft Derek Atkins het volgende geschreven:

Hi,
A few comments..

First, nice job, and thank you.
Second.. you don't need to change the name of the report. We should keep the same name, even though you're adding a new function. This way people with existing report configuration will get the new report functionality
automagically.
I Agree, so its back to 'advanced-portfolio'.

Third.. You should base the report on the one in 2.2.x (or SVN trunk),
not 1.8.  1.8 is two major revisions behind and, well, there've been
lots of changes and bugfixes that would get re-introduced by reverting
to 1.8
Started out yesterday from the current SVN version.

Fourth.. You should try to reduce the white-space changes; it makes
your changes harder to read and audit.  Worst case you could just
send in a diff ignoring white-space changes (see below).
Yeah, yeah, using DrScheme it gets obsessive-compulsive to re-indent the source in full though. :-)

So far it looks pretty good...  Any chance you could make sure the
report is based on 2.2 and send in a patch without whitespace changes?
You can use "svn diff -x -b" to ignore white space changes.
My svn diff does not accept the -x -b switches (svn, version 1.3.2 (r19776) compiled Jun 5 2006, 13:13:11) So created the (attached) diff as follows using GNU Diff (on Mac OS X 10.4.10): svn diff --diff-cmd /usr/bin/diff -x "-b" -r 81:83 ~/Documents/svn/ my_gnucash_reports/branches/advanced-portfolio.scm > ~/Desktop/diff.txt This results in < and > indicator instead of the - and + as in svn diff. Hope this is ok too?

Testresults not solved / explained:
1. with no pricelist data, I expected the transaction price to be shown, which didn't. Original report behaviour is same. I guess I don't quite understand the txn pricing. 2. selecting price source "Most recent to report" results in "337:29: Wrong type to apply: #<unspecified>". Same in original report (322:25: Wrong type to apply: #<unspecified>)

That's it for now.
Index: 
/Users/johan/Documents/svn/my_gnucash_reports/branches/advanced-portfolio.scm
===================================================================
41a42
> (define optname-sort-column (N_ "Sort Column"))
67c68,88
<      options gnc:pagename-general (N_ "Report Currency") "c")
---
>      options gnc:pagename-general (N_ "Report Currency") "b")
>     
>     (add-option
>      (gnc:make-multichoice-option
>       gnc:pagename-general optname-sort-column
>       "c" (N_ "The column on which the report is sorted") '0
>       (list (vector '0 (N_ "Account") (N_ ""))
>             (vector '1 (N_ "Symbol") (N_ ""))
>             (vector '2 (N_ "Listing") (N_ ""))
>             (vector '3 (N_ "Shares") (N_ ""))
>             (vector '4 (N_ "Price") (N_ ""))
>             (vector '5 (N_ "Tick") (N_ "")) ;; use-txn tick
>             (vector '6 (N_ "Basis") (N_ ""))
>             (vector '7 (N_ "Value") (N_ ""))
>             (vector '8 (N_ "Money In") (N_ ""))
>             (vector '9 (N_ "Money Out") (N_ ""))
>             (vector '10 (N_ "Realized Gain") (N_ ""))
>             (vector '11 (N_ "Unrealized Gain") (N_ ""))
>             (vector '12 (N_ "Total Gain") (N_ ""))
>             (vector '13 (N_ "Total Return") (N_ ""))
>             )))
105d125
< 
112a133
>     ;; Show Tab
147c168
<     ;; Account tab
---
>     ;; Account Tab
176d196
< 
280,281c300,301
<   
< (define (table-add-stock-rows table accounts to-date
---
>     ;; return list with computed values for all selected stocks
>     (define (table-add-stock-rows accounts to-date
287,290c307
<    (let ((share-print-info
<         (gnc-share-print-info-places
<          (inexact->exact (get-option gnc:pagename-display
<                                      optname-shares-digits)))))
---
>       (let ()
292,295c309,311
<     (define (table-add-stock-rows-internal accounts odd-row?)
<       (if (null? accounts) total-value
<           (let* ((row-style (if odd-row? "normal-row" "alternate-row"))
<                  (current (car accounts))
---
>         (define (table-add-stock-rows-internal accounts)
>           (if (null? accounts) (list) ;; return empty list
>               (let* ((current (car accounts))
304c320
< ;;                 (totalunits 0.0) ;;      these two items do nothing, but 
are in a debug below, 
---
>                      ;;                 (totalunits 0.0) ;;      these two 
> items do nothing, but are in a debug below, 
315d330
< 
332,333c347
< 
< ;;          (gnc:debug "---" name "---")
---
>                 ;;          (gnc:debug "---" name "---")
379,383c393,397
< ;;                       (gnc:debug "amount " (gnc-numeric-to-double 
(xaccSplitGetAmount s))
< ;;                                  " acct " (xaccAccountGetName 
(xaccSplitGetAccount s)) )
< ;;                       (gnc:debug "value " (gnc-numeric-to-double 
(xaccSplitGetValue s))
< ;;                                  " in " (gnc-commodity-get-printname 
commod-currency)
< ;;                                  " from " (xaccTransGetDescription 
(xaccSplitGetParent s)))
---
>                                  ;;                       (gnc:debug "amount 
> " (gnc-numeric-to-double (xaccSplitGetAmount s))
>                                  ;;                                  " acct " 
> (xaccAccountGetName (xaccSplitGetAccount s)) )
>                                  ;;                       (gnc:debug "value " 
> (gnc-numeric-to-double (xaccSplitGetValue s))
>                                  ;;                                  " in " 
> (gnc-commodity-get-printname commod-currency)
>                                  ;;                                  " from " 
> (xaccTransGetDescription (xaccSplitGetParent s)))
387,403c401,417
< ;; these lines do nothing, but are in a debug so I'm leaving it, just in 
case. asw.                        
< ;;                          (if (< 0 (gnc-numeric-to-double
< ;;                                    (xaccSplitGetAmount s)))
< 
< 
< ;;                              (set! totalunits
< ;;                                    (+ totalunits
< ;;                                       (gnc-numeric-to-double 
(xaccSplitGetAmount s))))
< ;;                              )
< 
< 
< ;;                          (set! totalunityears
< ;;                                (+ totalunityears 
< ;;                                   (* (gnc-numeric-to-double 
(xaccSplitGetAmount s))
< ;;                                      (gnc:date-year-delta 
< ;;                                       (car 
(gnc-transaction-get-date-posted parent))
< ;;                                       (current-time))))) 
---
>                                     ;; these lines do nothing, but are in a 
> debug so I'm leaving it, just in case. asw.                            
>                                     ;;                              (if (< 0 
> (gnc-numeric-to-double
>                                     ;;                                        
> (xaccSplitGetAmount s)))
>                                     
>                                     
>                                     ;;                                  (set! 
> totalunits
>                                     ;;                                        
> (+ totalunits
>                                     ;;                                        
>    (gnc-numeric-to-double (xaccSplitGetAmount s))))
>                                     ;;                                  )
>                                     
>                                     
>                                     ;;                              (set! 
> totalunityears
>                                     ;;                                    (+ 
> totalunityears 
>                                     ;;                                       
> (* (gnc-numeric-to-double (xaccSplitGetAmount s))
>                                     ;;                                        
>   (gnc:date-year-delta 
>                                     ;;                                        
>    (car (gnc-transaction-get-date-posted parent))
>                                     ;;                                        
>    (current-time))))) 
428,429c442,443
< ;;          (gnc:debug "totalunits" totalunits)
< ;;          (gnc:debug "totalunityears" totalunityears)
---
>                 ;;          (gnc:debug "totalunits" totalunits)
>                 ;;          (gnc:debug "totalunityears" totalunityears)
447d460
< 
465,466c478
< 
< 
---
>                 (gnc-price-list-destroy price-list)
469c481,482
<           (let* ((moneyin (gnc:monetary-neg
---
>                     (let*
>                         ((moneyin (gnc:monetary-neg
478c491,492
<                 (bothgain (gnc:make-gnc-monetary currency  (gnc-numeric-add 
(gnc:gnc-monetary-amount gain)
---
>                          (bothgain (gnc:make-gnc-monetary currency
>                                                           (gnc-numeric-add 
> (gnc:gnc-monetary-amount gain)
481,482d494
< 
<                 (activecols (list (gnc:html-account-anchor current)))
484d495
< 
492,498c503,507
<             ;; build a list for the row  based on user selections
<             (if show-symbol (append! activecols (list ticker-symbol)))
<             (if show-listing (append! activecols (list listing)))
<             (if show-shares (append! activecols (list 
(gnc:make-html-table-header-cell/markup
<               "number-cell" (xaccPrintAmount units share-print-info)))))
<             (if show-price (append! activecols (list 
(gnc:make-html-table-header-cell/markup
<               "number-cell"
---
>                       ;; build the list with all the computed values
>                       (cons (list current
>                                   ticker-symbol
>                                   listing
>                                   units
499a509,561
>                                       price
>                                       (gnc:make-gnc-monetary
>                                        (gnc-price-get-currency price)
>                                        (gnc-price-get-value price))) ;; price 
> (used in sort)
>                                   (if use-txn "*" " ") ;; use-txn Tick
>                                   (gnc:make-gnc-monetary currency (sum-basis 
> basis-list)) ;; basis
>                                   value
>                                   moneyin
>                                   moneyout
>                                   gain
>                                   ugain
>                                   bothgain                                    
>                 
>                                   (let ((moneyinvalue (gnc-numeric-to-double
>                                                        
> (gnc:gnc-monetary-amount moneyin))))
>                                     (if (= 0.0 moneyinvalue)
>                                         moneyinvalue
>                                         (* 100 (/ (gnc-numeric-to-double
>                                                    (gnc:gnc-monetary-amount 
> bothgain))
>                                                   moneyinvalue)))) ;; return
>                                   price
>                                   use-txn
>                                   pricing-txn
>                                   )
>                             (table-add-stock-rows-internal rest)))
>                     (table-add-stock-rows-internal rest)))
>               ))
>         
>         (set! work-to-do (gnc:accounts-count-splits accounts)) ;; #splits as 
> progress indicator
>         
>         (table-add-stock-rows-internal accounts)
>         
>         ))
>     
>     
>     ;; add one row with stock-computed values to the HTML table
>     (define (table-add-stock-row-html table share-print-info odd-row?
>                                       show-symbol show-listing show-shares 
> show-price
>                                       current ticker-symbol listing units
>                                       rate use-txn-tick basis value money-in 
> money-out gain ugain bothgain return price use-txn pricing-txn )
>       ;;                                      use-txn-tick price value 
> money-in money-out gain ugain bothgain return txnprice)
>       (let* ((row-style (if odd-row? "normal-row" "alternate-row"))
>              (odd-row? (not odd-row?))
>              (mycols (list (gnc:html-account-anchor current)))
>              )
>         ;; build the table-row (list) based on user selections
>         (if show-symbol (append! mycols (list ticker-symbol)))
>         (if show-listing (append! mycols (list listing)))
>         (if show-shares (append! mycols
>                                  (list (gnc:make-html-table-header-cell/markup
>                                         "number-cell" (xaccPrintAmount units 
> share-print-info)))))
>         (if show-price (append! mycols
>                                 (list (gnc:make-html-table-header-cell/markup
>                                        "number-cell" (if use-txn
509,512c571,573
<                   )))))
<             (append! activecols (list (if use-txn "*" " ")
<                                       (gnc:make-html-table-header-cell/markup 
<                                        "number-cell" (gnc:make-gnc-monetary 
currency (sum-basis basis-list)))
---
>                                                          ))))) ;; price
>         (append! mycols (list use-txn-tick
>                               (gnc:make-html-table-header-cell/markup 
> "number-cell" basis)
514,515c575,576
<                                       (gnc:make-html-table-header-cell/markup 
"number-cell" moneyin)
<                                       (gnc:make-html-table-header-cell/markup 
"number-cell" moneyout)
---
>                               (gnc:make-html-table-header-cell/markup 
> "number-cell" money-in)
>                               (gnc:make-html-table-header-cell/markup 
> "number-cell" money-out)
519,531c580
<                                                                               
<                                                                               
<                                       (gnc:make-html-table-header-cell/markup 
"number-cell" 
<                                           (let ((moneyinvalue 
(gnc-numeric-to-double
<                                                                
(gnc:gnc-monetary-amount moneyin))))
<                                             (if (= 0.0 moneyinvalue)
<                                                 (sprintf #f "%.2f%%" 
moneyinvalue)
<                                                 (sprintf #f "%.2f%%" (* 100 
(/ (gnc-numeric-to-double
<                                                                            
(gnc:gnc-monetary-amount bothgain))
<                                                                           
moneyinvalue))))))
<                                        )
<                       )
<                        
---
>                               (gnc:make-html-table-header-cell/markup 
> "number-cell" (sprintf #f "%.2f%%" return))))
535,542c584
<              activecols)
<               
<             (table-add-stock-rows-internal rest (not odd-row?))
<             )
<           (table-add-stock-rows-internal rest odd-row?)
<             )
<             (gnc-price-list-destroy price-list)
<           )))
---
>          mycols)))
544,545c586,641
<     (set! work-to-do (gnc:accounts-count-splits accounts))
<     (table-add-stock-rows-internal accounts #t)))
---
>     ;; add all computed values off all stocks selected to the HTML table
>     (define (table-add-stock-rows-html table
>                                        show-symbol show-listing show-shares 
> show-price
>                                        account-totals)
>       (let* 
>           ;; get printing related options first
>           ((share-print-info (gnc-share-print-info-places
>                               (inexact->exact (get-option gnc:pagename-display
>                                                           
> optname-shares-digits))))
>            (c (get-option gnc:pagename-general
>                           optname-sort-column))
>            (odd-row? #t))
>         (for-each
>          (lambda (l)
>            (apply table-add-stock-row-html table share-print-info odd-row?
>                   show-symbol show-listing show-shares show-price l))
>          ;; sort column (c in sort-list compare-less procedure) offsets are:
>          ;;  0 account
>          ;;  1 symbol
>          ;;  2 listing
>          ;;  3 shares (units)
>          ;;  4 price (rate)
>          ;;  5 use-txn Tick
>          ;;  6 basis
>          ;;  7 value
>          ;;  8 money-in
>          ;;  9 money-out
>          ;; 10 realized gain
>          ;; 11 unrealized gain
>          ;; 12 total gain
>          ;; 13 total return
>          (sort-list account-totals 
>                     (cond 
>                       ((= c 0)
>                        (lambda (list1 list2)
>                          (if (string<? (xaccAccountGetName (list-ref list1 c))
>                                        (xaccAccountGetName (list-ref list2 
> c))) #t #f)))
>                       ((or (and (> c 0) (< c 3)) (= c 5))
>                        (lambda (list1 list2)
>                          (if (string<? (list-ref list1 c) (list-ref list2 c)) 
> #t #f)))
>                       ((= c 3)
>                        (lambda (list1 list2)
>                          (if (< (gnc-numeric-to-double(list-ref list1 c))
>                                 (gnc-numeric-to-double(list-ref list2 c))) #t 
> #f)))
>                       ((or (= c 4) (= c 6) (= c 7))
>                        (lambda (list1 list2)
>                          (if (< (if (list-ref list1 c) (gnc-numeric-to-double 
> (gnc:gnc-monetary-amount (list-ref list1 c))) 0.0)
>                                 (if (list-ref list2 c) (gnc-numeric-to-double 
> (gnc:gnc-monetary-amount (list-ref list2 c))) 0.0)) #t #f)))
>                       ((and (> c 6) (< c 13))
>                        (lambda (list1 list2)
>                          (if (< (gnc-numeric-to-double 
> (gnc:gnc-monetary-amount (list-ref list1 c)))
>                                 (gnc-numeric-to-double 
> (gnc:gnc-monetary-amount (list-ref list2 c)))) #t #f)))
>                       ((> c 12)
>                        (lambda (list1 list2)
>                          (if (< (list-ref list1 c) (list-ref list2 c)) #t 
> #f)))
>                       )))))
584c680
<       ;;document will be the HTML document that we return.
---
>           ;; document will be the HTML document that we return.
594c690
<         ; at least 1 account selected
---
>           ;; at least 1 account selected
616c712
<         ;;begin building lists for which columns to display
---
>             ;; begin building lists for which columns to display
649,650c745,748
<           (table-add-stock-rows
<            table accounts to-date currency price-fn exchange-fn
---
>             (table-add-stock-rows-html table
>                                        show-symbol show-listing show-shares 
> show-price
>                                        (table-add-stock-rows accounts to-date
>                                                              currency 
> price-fn exchange-fn
652,653c750,751
<          basis-method prefer-pricelist total-basis total-value total-moneyin 
total-moneyout total-gain total-ugain)
<         
---
>                                                              basis-method 
> prefer-pricelist
>                                                              total-basis 
> total-value total-moneyin total-moneyout total-gain total-ugain))
657c755,756
<         (set! sum-total-both-gains (gnc:make-gnc-monetary currency 
(gnc-numeric-add (gnc:gnc-monetary-amount sum-total-gain)
---
>             (set! sum-total-both-gains (gnc:make-gnc-monetary currency 
>                                                               
> (gnc-numeric-add (gnc:gnc-monetary-amount sum-total-gain)
696d794
< 
700,701c798
<            totalscols
<             )
---
>              totalscols)
703a801
>             
709c807
< )
---
>             )
711c809
<                                       ;if no accounts selected.
---
>           ;; if no accounts selected

Regards,
Johan

Thanks!
-derek
Quoting Johan van Oostrum <[EMAIL PROTECTED]>:
Begin doorgestuurd bericht:
Van: Johan van Oostrum <[EMAIL PROTECTED]>
Datum: 11 september 2007 22:42:35 GMT+02:00
Aan: Derek Atkins <[EMAIL PROTECTED]>
Onderwerp: Antw.: GnuCash 2.2.x and advanced-report-sorted
Op 11-sep-2007, om 15:37 heeft Derek Atkins het volgende geschreven:
Hi,
What is the difference between this report and the existing
"advanced portfolio" report?
Functional change: sortcolumn option added
Code is (more or less) restructured to allow for easy use of sort
Would it be worthwhile to combine
them into a single report where the "sorting" part is just
a report option?  Or are the reports so significantly
different that merging them would be nearly impossible?
Excellent suggestion. I created a (my first :-) svn diff today.
If you can combine them into a single report with an option
to add sorting, sending in a patch (svn diff) would be a good
way to get it applied to SVN for the next release....
Attached you find the diff.
Notes
- name changed to advanced-portfolio-sorted
- it is an update of the GnuCash 1.8.x advanced report, not the new report that is packed with GC 2.2.x.
--
      Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
      Member, MIT Student Information Processing Board  (SIPB)
      URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
      [EMAIL PROTECTED]                        PGP key available

_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Reply via email to