Re: [Musicpd-dev-team] The libmpdclient v2.0 project
On 2008/12/01 00:28, Marc Pavot [EMAIL PROTECTED] wrote: 11 other patches available on my repository: Add unit tests to libmpclient No recursive makefiles, please. The patch contains 1 whitespace error. Fix glib include Change include guards Convert MPD_INFO_ENTITY_ to enum Applied. Fix currentsong test Fold this one into Add unit tests to libmpclient. Cosmetic Applied. Improve calls to mpd_executeCommand Don't do several independent changes in one patch, under the meaningless subject Improved xyz. The correct data type for string lengths is size_t, not int. Wrap lines which are longer than 79 characters (rare exceptions allowed, not applicable here). Small performance improvements Applied, but I removed the vacant summary Small performance improvements, and replaced it with the better description Avoid useless strcpy and useless strcmp. Split libmpdclient: entity Applied. Bug fix Applied, but removed meaningless subject line again. Split connection into connection ipc All structs will become opaque (as I have mentioned earlier). There is no need to introduce an opaque (sturct mpd_ipc) and a non-opaque (struct mpc_connection) part, unless you justify that with other means (which your patch description didn't). What do you mean by ''libmpdclient should be stateless regarding its caller program'? Do you plan to remove the getNext* commands and to make the send* commands return data? In this case, how do you want to implement CommandList'? libmpdclient knows the state of the connection and the server (i.e. waiting for response? Sending a command list?). But it should not have any assumptions on how the application processes libmpdclient's responses (regarding its caller program). Callbacks limit the way you may use libmpdclient; the high-level API (which is yet to be designed) may callbacks. Max - 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=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
11 other patches available on my repository: Add unit tests to libmpclient Fix glib include Change include guards Convert MPD_INFO_ENTITY_ to enum Fix currentsong test Cosmetic Improve calls to mpd_executeCommand Small performance improvements Split libmpdclient: entity Bug fix Split connection into connection ipc What do you mean by ''libmpdclient should be stateless regarding its caller program'? Do you plan to remove the getNext* commands and to make the send* commands return data? In this case, how do you want to implement CommandList'? Marc - 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=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
- I saw that your idle patch introduced inconsistent indentation (e.g. mpd_glibStartIdle); please configure your editor to indent with tabs. I don't want to start a 'religion war' on this point but don't you think it would be better to use spaces everywhere instead of tabs? This discussion has already been done thousands of times by other people so I won't say more but you can find a lot of projects using spaces instead of tabs because tabs are always a nightmare for projects involving different developers and for developers contributing to several projects. (spaces indent makes you sure that the files will appear identically in any text editor). - A commit should include a brief one-line description, followed by an empty line, and a longer description following. This way, stuff like git shortlog works properly. Don't write another description in the email - a well-documented patch doesn't need further explanation. Ok. It's my first time using git but I will remember it. - set local includes first, to test if the include file has specified its own dependencies properly, i.e. include idle.h before stdio.h and string.h in idle.c. In return_element.c you did this very well: return_element.h before str_pool.h (str_pool.h was already checked by str_pool.c). Ok. - 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=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
On 2008/11/27 12:10, Marc Pavot [EMAIL PROTECTED] wrote: I don't want to start a 'religion war' on this point but don't you think it would be better to use spaces everywhere instead of tabs? Personally, I prefer spaces, too, but let's just focus on important technical problems. The current code uses tabs, and let's keep it this way (consistently). Max - 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=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
Hi, There are some commits on my repository that you may find interesting to integrate. Marc Pavot: Use tabs instead of spaces Split libmpdclient: status Set local includes first for idle.c Split libmpdclient: stats Merge branch 'master' of git://git.musicpd.org/cirrus/libmpdclient Merge branch 'master' of git://git.musicpd.org/cirrus/libmpdclient Remove useless malloc/free changed typedef mpd_Status to struct changed typedef mpd_Stats and mpd_SearchStats to struct New disconnect event W32 support of idle mode Marc - 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=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
On 2008/11/28 00:49, Marc Pavot [EMAIL PROTECTED] wrote: There are some commits on my repository that you may find interesting to integrate. Thanks, I've merged all. New disconnect event + IDLE_DISCONNECT = 0x80, This is a clumsy choice, since it will conflict with the next event which may be added to MPD; if you change the numbers, the libmpdclient ABI is changed. Nobody forces you to sync these numbers with MPD, but it would be nice, because you can just copy'n'paste the enum. Maybe the special value 0x0 would be a better choice here; still not elegant, but ok for now. I think we will review the idle API when we add real non-blocking I/O. I'm not comfortable with callback functions here, libmpdclient should be stateless regarding its caller program. Callbacks could be implemented in the higher-level libmpdclient part. By the way, we should add the MPD_ prefix to these constants some day. Max - 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=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
- portability - even Windows? There's no good C99 compiler on Windows.. mingw32? Just a quick remark about this point: I compile Ario and libmpdclient on windows with mingw32 and it works fine. (I distribute .exe files for Ario). - 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=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
Some of you already promised to help me with that. From now on, I will accept patches (git pull requests preferred). Please keep your git repository rebased on mine. I have created a git repository to help you on this project: http://repo.or.cz/w/libmpdclient/marc.git It is based on your repository and I have done one commit to continue the split of libmpdclient. It creates three new files: connection, idle and return_element. The separation is not perfect (for example some structures and methods in connection will have to become private in the future) but it's a first brick. Marc - 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=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
On 2008/11/27 00:33, Marc Pavot [EMAIL PROTECTED] wrote: It is based on your repository and I have done one commit to continue the split of libmpdclient. It creates three new files: connection, idle and return_element. The separation is not perfect (for example some structures and methods in connection will have to become private in the future) but it's a first brick. Thanks, it's merged. I have some suggestions: - I saw that your idle patch introduced inconsistent indentation (e.g. mpd_glibStartIdle); please configure your editor to indent with tabs. - A commit should include a brief one-line description, followed by an empty line, and a longer description following. This way, stuff like git shortlog works properly. Don't write another description in the email - a well-documented patch doesn't need further explanation. - set local includes first, to test if the include file has specified its own dependencies properly, i.e. include idle.h before stdio.h and string.h in idle.c. In return_element.c you did this very well: return_element.h before str_pool.h (str_pool.h was already checked by str_pool.c). Max - 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=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
