On Sat 26 Jan 2013 11:15, Nala Ginrut <nalagin...@gmail.com> writes: > Please review it. ;-P
A drive-by review (i.e., just style comments and random questions) > ;; Copyright (C) 2012 Free Software Foundation, Inc. 2013 > ;;;; Author: Mu Lei known as NalaGinrut <nalagin...@gmail.com> > (define-module (ice-9 colorized) > #:use-module (ice-9 rdelim) > #:use-module ((srfi srfi-1) #:select (filter-map any proper-list?)) > #:use-module (srfi srfi-9) > #:use-module (system repl common) > #:export (activate-colorized custom-colorized-set! color-it colorize-it > colorize > colorize-string colorized-display add-color-scheme!)) No tabs, please. In emacs this is (indent-tabs-mode nil) I think; you can M-x untabify also. > (define (activate-colorized) > (let ((rs (fluid-ref *repl-stack*))) > (if (null? rs) > (repl-default-option-set! 'print colorized-repl-printer) ; if no REPL > started, set as default printer > (repl-option-set! (car rs) 'print colorized-repl-printer)))) ; or set > as the top-REPL printer Nice > (define *color-list* > `((CLEAR . "0") Why are these upper-cased? Unless there is a reason, lower-case is better as it is easier to type and read. > (define (get-color color) > (assoc-ref *color-list* color)) Use assq-ref, the keys are symbols > (define (generate-color colors) > (let ((color-list > (filter-map (lambda (c) (assoc-ref *color-list* c)) colors))) Use get-color here; and is the intention to silently ignore unknown colors? > (define (colorize-the-string color str control) > (string-append (generate-color color) str (generate-color control))) The name is confusingly like "colorize-string" below. Better to have a more descriptive name, or maybe colorize-string-helper. > ;; test-helper functions > ;; when eanbled, it won't output colored result, but just normal. > ;; it used to test the array/list/vector print result. > (define *color-func* (make-fluid colorize-the-string)) > (define (disable-color-test) > (fluid-set! *color-func* colorize-the-string)) > (define (enable-color-test) > (fluid-set! *color-func* color-it-test)) Surely testing-related functions should not be here? > (define (color-it-inner color str control) > ((fluid-ref *color-func*) color str control)) Use parameters instead of fluids. > (define *pre-sign* > `((LIST . "(") > (PAIR . "(") > (VECTOR . "#(") > (ARRAY . #f))) > ;; array's sign is complecated, return #f so it will be handled by pre-print "complicated" > (define* (pre-print cs #:optional (port (current-output-port))) > (let* ((type (color-scheme-type cs)) > (control (color-scheme-control cs)) > (sign (assoc-ref *pre-sign* type)) assq-ref > (define *colorize-list* > `((,integer? INTEGER ,color-integer (BLUE BOLD)) > (,char? CHAR ,color-char (YELLOW)) > (,string? STRING ,color-string (RED)) Interesting :) Regards, Andy -- http://wingolog.org/