Re: [emms-help] [patch] move tracks in playlist

[Yoni Rabkin]

Rasmus <address@hidden> writes:

> 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
emms-playlist-mode. Read on below:

> 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.

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)

> Rasmus
>
> -- 
> Together we will make the possible totalllly impossible!
>
>>From 4c0be3c9ecf66d7fb2dfeac56158fcf89aecf8eb Mon Sep 17 00:00:00 2001
> From: Rasmus <address@hidden>
> 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
> strings), a number of functions were changed:
>  - 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
> @@ -9,6 +9,7 @@ News since version 4.0:
>    - 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.
>  
>  News since version 3.0:
>  
> 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
> address@hidden emms-playlist-move-track-up
> +Move the current track up in the playlist.
> address@hidden emms-playlist-move-track-down
> +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.
> address@hidden M-down
> address@hidden emms-playlist-move-track-down
> +Move the current track down in the playlist.
>  @item p
>  @findex emms-next
>  Start playing the previous track in the playlist.
> address@hidden M-up
> address@hidden emms-playlist-move-track-up
> +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"