On Fri, 2013-01-11 at 16:13 +0800, Daniel Hartwig wrote: > On 11 January 2013 14:29, Nala Ginrut <nalagin...@gmail.com> wrote: > > On Thu, 2013-01-10 at 16:19 +0800, Daniel Hartwig wrote: > > I changed these: > > string-in-color => colorize-string > > display-string-in-color => colorized-display > > > > What do you think? > > Nicer anway. > > >> Also, the “/” in “1/2” appears as a different colour, why?! > >> > > > > It's more conspicuous for the users. I asked some guys, they like it. > > > > I see. > > > > > But how to check if a REPL is already running? > > I believe there was some fluid referenced by a previous version of > activate-colorized. > > >> Procedure arguments “data” which should rather be “obj”. > >> > > > > Colorize an 'obj' is strange, I think 'data' is better. > > > > Not strange at all in the context of Scheme where this is a well > established convention. “obj” literally means “any object”, talking > about Scheme objects (not GOOPS). “data” is not commonly used and > when it is, typically has a very specific meaning that does not > include “any object”. To read a procedure definition that uses the > name “obj” I can immediately tell what types of values it expects > (anything!), whereas “data” carries no particular hints at all. > > In any Scheme documentation, such as the Guile manual or any Revised* > Report on Scheme, there is almost exclusive use of the name “obj” for > arguments like this. See the R5RS section “Entry format”. >
OK~fixed. > > But what's the expected result of "(write #u8(0 1 2)) (newline)"? > > As I wrote: #u8(0 1 2). I provided that line to demonstrate that > colorize-it was producing different output. > > > > >> The chance of subtle problems with other data types is high, both now > >> and in the future after any current problems are corrected. > >> > > > > Any code contains potential bugs. > > Yes. > > > I don't think people should use (ice-9 colorize) in their serious > > program, it's just a tool for newbies to learn Guile more friendly. Even > > 'colorize-string', it's the co-product of that, and it just output a > > string in color, simple enough to avoid dangerous. > > Regardless of the purpose, if it is shipped as part of Guile then it > will be used. It's output must be reliable. > > Keep in mind that you can not anticipate everything people will do > with code. It takes on it's own life and you can only smile and watch > it play :-) > > > > > But it's inefficient if I remove ansi-code each time after it's > > generated. I prefer output the plain string on the fly with enable test > > option. > > Test cases do not have to be efficient, they must prove the real code. > By replacing the innermost procedure—colorize-the-string—with a > testing variant you are no longer testing the real code, but something > else. > Yes, that's a good point, and the test case could move out of the module itself. > >> I agree with Ludo, the string and color scheme are not so related. > >> This design choice adds confusion to the rest of the module. > >> > > > > OK, I think it's an absolute design, fixed. > > I suppose the original comments were not so clear. It is not only the > string but other members such as “data” that do not fit the concept of > “colour scheme”. Anyway, given that it is an internal type there is > not much point to restructuring it all, except for pedantics. > Well, if it's too uncomfortable, maybe it should rename to "colorize-context". Anyway, I agree with your opinion, it's not so important for an internal type. > > >> when, if “colorize” produced strings, it could do something like this: > >> > > > Nice~I'm in the first way, and added a helper function '->cstr' to > > generate color string result for any type. > > A good hack I like it~ > > Indeed. The new loops are certainly an improvement. > > Personally I would have avoided call-with-output-string in ->cstr and > other low-level procedures, since now there are several calls in and > out of string ports. I suppose that is another early design decision > that is not worth the effort to change. > Yes, I could do that but taking other advantage of others functions is better, and it introduced call-with-outout-string anyway. > > > >> >> > +(define color-array-inner > >> > >> >> Wow, this is hairy and heavyweight. > >> > >> > Yes, but the aim of colorized-REPL is to show more friendly UI to the > >> > users, it dropped up some efficiency designs. > >> > >> Disagree. It is more difficult to read the array tag with all the > >> colour changes. Breaking apart elements to this level is extreme, > >> prone to errors, and poorly maintainable. All that for a very > >> questionable gain in “friendly UI”. Keep things simple, at least > >> until you have a smooth module without issues. > >> > > > > Even if I don't break apart it, it's inefficient too, I have to convert > > it to string then output the prefix-part. > > You could inspect the array and generate the tag yourself. This would > not explode on very large data structures. But, as you go on to say … > Seems it's the only 'suck point' now, I do try to handle the prefix with Guile's function, but the Array is too complicated (and not the srfi-63 compatible), finally I lost, so I tried a easy solution... PS: I don't think srfi-63 is matter here, just to say it's a brand new complicated thing to learn... > > As I said, nobody will use it in one's serious program, since it's only > > about REPL. Who care about the REPL running speed? Users like a more > > friendly REPL UI not a quick REPL since it's useless in a released > > program but for test/debug. Except for 'colorized-string', but it's a > > simple function which is safe and fast. > > Indeed, with this outlook there is little point to such alternative > treatment, and the easiest thing to do is to rely on the existing > procedures to produce the correct output. > If the Array is the only problem, we'd better fix it anyway. I think there's no other bad thing left, or not? > However I will say that an efficient and flexible implementation would > certainly be useful in applications outside of the developers REPL. > Applications display data, and often contain REPL and equivalent. :-) > > > Please review it: > > https://github.com/NalaGinrut/guile-colorized/tree/upstream > > > > This: > > (define colorize-the-string > (lambda (color str control) > (string-append "\x1b[" (generate-color color) "m" str > "\x1b[" (generate-color control) "m"))) > > Why move "\x1b[" and "m" out of generate-color here? Have that > procedure procedure the /complete/ escape sequence and it is much > neater. > Ah~yeah, fixed. > Regards > I'll call this 'sculpture hacking', hah? ;-D Thanks!
