Discussion:
[emms-help] [patch] move tracks in playlist
Rasmus
2015-05-24 12:58:46 UTC
Permalink
Hi,

This patch adds support for moving tracks in the playlist.

Since I change other playlist functions (not user-visible, I think), I'll
wait a couple of days before merging to see if anybody is opposed to any
changes.

A small rant:

The playlist is a mess. Faces differ depending on how they were inserted
into the buffer. The displayed string depends on how it was inserted.
The indentation is also a function of insertion method.

One thing I would be cool would be use tabulated list (like packaged mode)
and have /one/ format-string that controls how items look and are aligned.
I'd have to check if cover art can work with this.

Rasmus
--
Together we will make the possible totalllly impossible!
Yoni Rabkin
2015-05-24 21:55:52 UTC
Permalink
Post by Rasmus
Hi,
This patch adds support for moving tracks in the playlist.
Since I change other playlist functions (not user-visible, I think), I'll
wait a couple of days before merging to see if anybody is opposed to any
changes.
Thank you for posting this before inclusion. We need to talk about it
first.

I'm afraid that having separate track and track-string arguments to the
insert function would be inviting a lot of breakage and confusion down
the road, and separate M-up and M-down keys don't make sense. This isn't
a problem with your code but with the diverging use cases for
Post by Rasmus
The playlist is a mess. Faces differ depending on how they were inserted
into the buffer. The displayed string depends on how it was inserted.
The indentation is also a function of insertion method.
One thing I would be cool would be use tabulated list (like packaged mode)
and have /one/ format-string that controls how items look and are aligned.
I'd have to check if cover art can work with this.
The playlist wasn't designed to display covers, different faces, have
indentation, etc. This is because you are trying to use
emms-playlist-mode for stuff that it wasn't designed for. The playlist
mode isn't messy, it's just that trying to shoehorn a graphical UI into
a mode which is a vertical list of textual lines is very messy. This is
by design.

I don't use the browser; I don't need or want anything but a single line
of non-fancy, non-indented simple text per track without pictures,
photos or covers etc.. I want my playlist to be as close to a regular
Emacs buffer with normal Emacs movements as possible (both visually and
in terms of key-bindings, if I wanted something else, I wouldn't be
using Emacs but a player with a graphical UI.)

The solution is either for you to create a separate playlist mode
designed from the ground up for the browser, or for me to create
something like emms-purelist-mode or something, which continues to serve
my purposes (and the purposes of other people with eye problems, who
need as little graphical fanciness as possible). In either case, after
the split, you would be able to make the changes you need without
unnecessarily overloading the playlist mode with exceptions and
additional code paths, and you won't need to worry about whether your
design is breaking anything for people like me who need to the playlist
buffer to remain the same.

Which one we do depends on whether you want the code from
emms-playlist-mode as a basis or not. I think that people like me are
the minority and therefore I have no issue with doing the work to make a
simple "purelist" mode which will remain pristine.

(and yes, I modify the rest of Emacs heavily for my visual needs, from
creating simple, hi-visibility colors, to removing almost all faces
except the ones I use to read)
Post by Rasmus
Rasmus
--
Together we will make the possible totalllly impossible!
From 4c0be3c9ecf66d7fb2dfeac56158fcf89aecf8eb Mon Sep 17 00:00:00 2001
Date: Sun, 24 May 2015 14:30:03 +0200
Subject: [PATCH] Add move functions to the playlist view
This allows moving track with M-up and M-down (like Org headings).
To facilitate a consistent behavior (e.g. not changing fonts or
- Kill functions take an optional no-stop argument
- emms-playlist-mode-insert-track takes a new optional
track-string argument that is used as string if non-nil.
Further, it tries to use the same face as track-string if present.
---
NEWS | 1 +
doc/emms.texinfo | 10 ++++++
lisp/emms-playlist-mode.el | 85 +++++++++++++++++++++++++++++++++++-----------
3 files changed, 77 insertions(+), 19 deletions(-)
diff --git a/NEWS b/NEWS
index 65c439b..40b45bd 100644
--- a/NEWS
+++ b/NEWS
- emms-lyrics.el now uses eww if present. Also EMMS tries to fetch
non-Chinese lyrics from lyricwiki.org.
- Add HTTPS support where possible.
+ - Tracks can be moved in the emms playlist using M-up and M-down.
diff --git a/doc/emms.texinfo b/doc/emms.texinfo
index 26bb10f..c0db3cd 100644
--- a/doc/emms.texinfo
+++ b/doc/emms.texinfo
@@ -681,6 +681,10 @@ Move to the previous track in the current buffer.
@defun emms-playlist-previous
Move to the previous track in the current buffer.
@end defun
+Move the current track up in the playlist.
+Move the current track down in the playlist.
@defun emms-random
Jump to a random track.
@end defun
@@ -1042,9 +1046,15 @@ Set the current playlist buffer.
@item n
@findex emms-next
Start playing the next track in the playlist.
+Move the current track down in the playlist.
@item p
@findex emms-next
Start playing the previous track in the playlist.
+Move the current track up in the playlist.
@item s
@findex emms-stop
Stop playing.
diff --git a/lisp/emms-playlist-mode.el b/lisp/emms-playlist-mode.el
index e8972c0..ee175a4 100644
--- a/lisp/emms-playlist-mode.el
+++ b/lisp/emms-playlist-mode.el
@@ -130,6 +130,8 @@ This is true for every invocation of `emms-playlist-mode-go'."
(define-key map (kbd "M->") 'emms-playlist-mode-last)
(define-key map (kbd "M-n") 'emms-playlist-mode-next)
(define-key map (kbd "M-p") 'emms-playlist-mode-previous)
+ (define-key map [(meta up)] 'emms-playlist-move-track-up)
+ (define-key map [(meta down)] 'emms-playlist-move-track-down)
(define-key map (kbd "a") 'emms-playlist-mode-add-contents)
(define-key map (kbd "b") 'emms-playlist-set-playlist-buffer)
(define-key map (kbd "D") 'emms-playlist-mode-kill-track)
@@ -341,6 +343,38 @@ set it as current."
(error "No track at point"))))
;;; --------------------------------------------------------
+;;; Move track
+;;; --------------------------------------------------------
+
+(defun emms-playlist-move-track-up (&optional n)
+ "Move track at point upwards in the playlist.
+Optional argument n is the number of spaces "
+ (interactive "p")
+ (let ((track (emms-playlist-track-at))
+ (playing-track (and emms-player-playing-p
+ (emms-playlist-selected-track-at-p)))
+ (string (buffer-substring (line-beginning-position)
+ (line-end-position)))
+ (n (- (or n 1))))
+ (emms-playlist-mode-kill-entire-track t)
+ (forward-line n)
+ (save-excursion
+ (emms-with-inhibit-read-only-t
+ (emms-playlist-mode-insert-track track nil string)
+ (emms-playlist-mode-correct-previous-yank)))
+ (and playing-track
+ (emms-with-inhibit-read-only-t
+ (emms-playlist-select (point))))))
+
+(defun emms-playlist-move-track-down (&optional n)
+ "Drag track at point downwards in the playlist.
+Optional argument n is the number of spaces "
+ (interactive "p")
+ (let ((n (- (or n 1))))
+ (emms-playlist-move-track-up n)))
+
+
+;;; --------------------------------------------------------
;;; Killing and yanking
;;; --------------------------------------------------------
@@ -350,15 +384,17 @@ set it as current."
(<= p b)))
;; D
-(defun emms-playlist-mode-kill-entire-track ()
+(defun emms-playlist-mode-kill-entire-track (&optional no-stop)
"Kill track at point, including newline."
(interactive)
(let ((kill-whole-line t))
- (emms-playlist-mode-kill-track)))
+ (emms-playlist-mode-kill-track no-stop)))
;; C-k
-(defun emms-playlist-mode-kill-track ()
- "Kill track at point."
+(defun emms-playlist-mode-kill-track (&optional no-stop)
+ "Kill track at point.
+Optional no-stop allows the track to continue playing.
+"
(interactive)
(emms-with-inhibit-read-only-t
(let ((track (emms-playlist-track-at)))
@@ -366,8 +402,8 @@ set it as current."
(let ((track-region (emms-property-region (point)
'emms-track)))
(when (and emms-player-playing-p
- (emms-playlist-selected-track-at-p))
- (emms-stop)
+ (emms-playlist-selected-track-at-p))
+ (unless no-stop (emms-stop))
(delete-overlay emms-playlist-mode-selected-overlay)
(setq emms-playlist-mode-selected-overlay nil))))
(let ((kill-whole-line emms-playlist-mode-kill-whole-line-p))
@@ -375,7 +411,7 @@ set it as current."
(kill-line)))))
;; C-w
-(defun emms-playlist-mode-kill ()
+(defun emms-playlist-mode-kill (&optional no-stop)
"Kill from mark to point."
(interactive)
(emms-with-inhibit-read-only-t
@@ -385,7 +421,7 @@ set it as current."
(marker-position emms-playlist-selected-marker)
(region-beginning)
(region-end)))
- (emms-stop)
+ (unless no-stop (emms-stop))
(delete-overlay emms-playlist-mode-selected-overlay)
(setq emms-playlist-mode-selected-overlay nil))
(kill-region (region-beginning)
@@ -481,18 +517,29 @@ This preserves the current EMMS buffer."
;;; Local functions
;;; --------------------------------------------------------
-(defun emms-playlist-mode-insert-track (track &optional no-newline)
+(defun emms-playlist-mode-insert-track (track &optional no-newline track-string)
"Insert the description of TRACK at point.
-When NO-NEWLINE is non-nil, do not insert a newline after the track."
+When NO-NEWLINE is non-nil, do not insert a newline after the track.
+When TRACK-STRING is non-nil it will be used as the playlist text.
+Otherwise `emms-track-force-description' is used."
(emms-playlist-ensure-playlist-buffer)
- (emms-with-inhibit-read-only-t
- (insert (emms-propertize (emms-track-force-description track)
- 'emms-track track
- 'face 'emms-playlist-track-face))
- (when (emms-playlist-selected-track-at-p)
- (emms-playlist-mode-overlay-selected))
- (unless no-newline
- (insert "\n"))))
+ (let ((face (and track-string
+ (or (get-text-property 0 'face track-string)
+ (get-text-property
+ (next-single-property-change 0 'face track-string)
+ 'face track-string)))))
+ (and track-string
+ (set-text-properties 0 (length track-string) nil track-string))
+ (emms-with-inhibit-read-only-t
+ (insert (emms-propertize (or track-string
+ (emms-track-force-description track))
+ 'emms-track track
+ 'face (or face 'emms-playlist-track-face)))
+ (when (save-excursion (beginning-of-line)
+ (emms-playlist-selected-track-at-p))
+ (emms-playlist-mode-overlay-selected))
+ (unless no-newline
+ (insert "\n")))))
(defun emms-playlist-mode-update-track-function ()
"Update the track display at point."
@@ -509,7 +556,7 @@ When NO-NEWLINE is non-nil, do not insert a newline after the track."
(when selectedp
(delete-overlay emms-playlist-mode-selected-overlay)
(setq emms-playlist-mode-selected-overlay nil))
- (emms-playlist-mode-insert-track track t))
+ (emms-playlist-mode-insert-trac track t))
(when selectedp
(emms-playlist-select (point))))))
--
"Cut your own wood and it will warm you twice"
Rasmus
2015-05-24 22:30:50 UTC
Permalink
Post by Yoni Rabkin
I'm afraid that having separate track and track-string arguments to the
insert function would be inviting a lot of breakage and confusion down
the road,
I can create a separate function if that's better. Or make the moving
function monolithic.
Post by Yoni Rabkin
and separate M-up and M-down keys don't make sense.
I don't understand this part. We can add support for moving tracks via
killing as well, Emacs way.

M-{up, down} is how stuff is moved around in Org.
Post by Yoni Rabkin
The playlist wasn't designed to display covers, different faces, have
indentation, etc. This is because you are trying to use
emms-playlist-mode for stuff that it wasn't designed for. The playlist
mode isn't messy, it's just that trying to shoehorn a graphical UI into
a mode which is a vertical list of textual lines is very messy. This is
by design.
OK. The fact that emms-add-... and that emms-browser-add (or whatever)
uses different functions is messy. There should be one function to add
tracks to the playlist to ensure consistency.
Post by Yoni Rabkin
I don't use the browser; I don't need or want anything but a single line
of non-fancy, non-indented simple text per track without pictures,
photos or covers etc.. I want my playlist to be as close to a regular
Emacs buffer with normal Emacs movements as possible (both visually and
in terms of key-bindings, if I wanted something else, I wouldn't be
using Emacs but a player with a graphical UI.)
In this case it's like an Org buffer. In any case, I respect your
decision.
Post by Yoni Rabkin
The solution is either for you to create a separate playlist mode
designed from the ground up for the browser, or for me to create
something like emms-purelist-mode or something, which continues to serve
my purposes (and the purposes of other people with eye problems, who
need as little graphical fanciness as possible). In either case, after
the split, you would be able to make the changes you need without
unnecessarily overloading the playlist mode with exceptions and
additional code paths, and you won't need to worry about whether your
design is breaking anything for people like me who need to the playlist
buffer to remain the same.
Would that not break emms-add-whatever? Anyway, it could be interesting
experiment.

Rasmus
--
Vote for proprietary math!
Yoni Rabkin
2015-05-24 23:12:09 UTC
Permalink
Post by Rasmus
Post by Yoni Rabkin
I'm afraid that having separate track and track-string arguments to the
insert function would be inviting a lot of breakage and confusion down
the road,
I can create a separate function if that's better. Or make the moving
function monolithic.
Post by Yoni Rabkin
and separate M-up and M-down keys don't make sense.
I don't understand this part. We can add support for moving tracks via
killing as well, Emacs way.
M-{up, down} is how stuff is moved around in Org.
This may be true, but orthogonal to the argument I'm making. I'm saying
that you can take the way the browser organizes it's songs in any
direction, including Org's, but that having to step carefully around
certain features of emms-playlist-mode as-is makes doing that a lot
harder than it needs to be.
Post by Rasmus
Would that not break emms-add-whatever? Anyway, it could be interesting
experiment.
No, it would probably be an issue of introducing another level of
indirection in the process.
--
"Cut your own wood and it will warm you twice"
Loading...