Re: [Musicpd-dev-team] ncmpc, mpd

2009-03-27 Thread Max Kellermann
On 2009/03/26 21:02, Jeffrey Middleton jefr...@gmail.com wrote:
 Some updates:
 git pull git://git.musicpd.org/jefromi/ncmpc.git master
   command: Changed key for screen swap to `
   NEWS: list commands added since release 0.13
   conf: screen-list fix, removed unused option
   man: configuration section updated
   conf: Allow descriptive search modes

I merged these patches.

 and something to go along with mpd move range
 git pull git://git.musicpd.org/jefromi/ncmpc.git w-range
   Implemented move command with range
 
 This one is not totally finished - it works, but doesn't check if
 the ranged move is available.

Right.  ncmpc must be able to work with any (non-ancient) MPD version.
But I think we don't need to use the ranged move at all here, because
ncmpc can't move by more than one position at a time - just send one
old-style move command.

Your libmpdclient changes look useful, we should add that to
libmpdclient.git (adapted to its new code style).

Max

--
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team


Re: [Musicpd-dev-team] ncmpc, mpd

2009-03-27 Thread Max Kellermann
On 2009/03/27 03:51, Jeffrey Middleton jefr...@gmail.com wrote:
 I thought it was worth clarifying part of the move range command: it moves
 the first song in the given range to the destination position, and the rest
 of the range follows it.  (This much is said in the commit message)

This is not what your code does.  Imagine move 0:10 1, if you'd move
this one by one, you would interchange swap the first two songs over
and over.

 This can be counterintuitive; for example, to move the range to the
 end of the playlist, the destination is the length of the range
 short of the end of the playlist.  Similar trickiness to move to a
 desired position between two existing songs.  This will mean some
 work on the client side, unfortunately.  However, the alternative -
 having the block move to just after (or before) the song in the
 indicated position - makes things as simple as moving the block down
 one difficult - you have to specify a position one after the end of
 the range, rather simply the position one greater than the starting
 position.  To sum up, I think this is the method that makes the most
 sense, but am quite open to suggestions.

I believe the approach you chose in your code was the best: the
destination position is the new position of the first item in the
range.  Everything else will be more complicated (to understand and to
implement), because of overlapping ranges and possible playlist
corruption if you do it wrong...

Max

--
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team


Re: [Musicpd-dev-team] ncmpc, mpd

2009-03-26 Thread Jeffrey Middleton
On Thu, Mar 26, 2009 at 4:15 PM, Max Kellermann m...@duempel.org wrote:


 Watch out for those compiler warnings, always work with
 --enable-werror:

  src/playlist_edit.c: In function 'moveSongRangeInPlaylist':
  src/playlist_edit.c:302: error: comparison between signed and unsigned
  src/playlist_edit.c:306: error: comparison between signed and unsigned
  src/playlist_edit.c:325: error: comparison between signed and unsigned

 I have fixed those warnings, and merged your patch.


Darn, I thought I was careful there.  I get warnings from libraries when
compiling mpd so I've not been using --enable-werror, and trying to check to
make sure there were no warnings.

Incidentally, with ncmpc on an Ubuntu 8.10 system with ncurses
5.6+20071124-1ubuntu2 I get warnings
colors.c:281: warning: the address of ‘attrs’ will never be NULL
colors.c:281: warning: the address of ‘pair’ will never be NULL
but not on my work computer (CentOS 5.0).  Not complaining, just noting that
unfortunately --enable-werror is a great idea that doesn't always serve me
very well.



 I'm going to review the ncmpc patches tomorrow.  Remind me just in
 case I forget...


I'll be gone by then, but I'll remind you next week!

Jeffrey
--
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team