concordance/concordance.1

-Learn IR from other remotes. Use <filename>.
+Learn IR from other remotes. Use <filename>. Supports also files with
multiple key names, i.e. when you checked multiple commands on the Logitech
website. For each command, you have the choice to skip to the next or
previous command or to learn the IR code from the original remote.
 .TP


I see no need to specify that learn-ir supports multiple commands. It should
have always, and most users are only going to be confused by this. This now
supports the format the site can send - it's lack of full support before was
simply a bug. In other words, I think this can stay as it was.


concordance/concordance.c

+       if( flag) {
+               t.c_lflag |= ICANON;

Minor spacing issue in the "if" there.

+#define read_key getchar

Please explain (in a comment) that your set_canon() function changes the
behavior of getchar() - this is not at all clear.

+#define USE_DEFAULT '\n'

This name is horrible, how about, NEWLINE or SYSTEM_NEWLINE?

Also, windows doesn't give '\r', it gives '\r\n' where as unix gives just
\n. So your stuff works kinda by accident, but that should probably be fixed.

+       /*
+               * full dump of new IR signal:
+               */

Typo?

+                                               "[U]pload new code, [R]etry
same key, [N]ext key, [Q]uit",

Does it make sense to maybe put this prompt string in a variable and use
that instead? I think it would make the code much more readable.

-                       err = learn_ir_commands(data, size, !options.noweb);
+                       err = _learn_ir_commands(data, size, &options);

This doesn't need to start with an underscore. None of the mode functions do.

In fact, nothing here should start with an underscore, it's not a library,
nothing's private. However, please do rename _printburst and _printspace and
_dump_new_code to something like print_ir_burst, print_ir_space, and
dump_ir_code.

Also, I notice you do index++ at the beginning, and then have all these
cases where it you index-- or index -= 2. It seems more logical to leave
index at the current index, and only ++ in the event of success. If you
fail, you don't do anything (nuking the --), if you get a "p" you just --.
Is there any reason not to change to this approach?

-- 
Phil Dibowitz                             [EMAIL PROTECTED]
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Never write it in C if you can do it in 'awk';
 Never do it in 'awk' if 'sed' can handle it;
 Never use 'sed' when 'tr' can do the job;
 Never invoke 'tr' when 'cat' is sufficient;
 Avoid using 'cat' whenever possible" -- Taylor's Laws of Programming


Attachment: signature.asc
Description: OpenPGP digital signature

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to