[bongo-devel] Re: Marks

[Daniel Jensen]

Daniel Brockman <address@hidden> writes:

> Let's also add `% u' for unmarking by regexp match, to go
> with `% m', and also `% A', `% B', `% T, `% Y' and `% I' for
> unmarking by regexp match against specific fields, to go
> with their lowercase variants.

I think using the prefix argument is better here. It is how it's done
in dired (but I missed it when I was testing it). Of course, you could
include these as well if you want.

Your other comments on bindings all make sense to me; I'll leave it to
Romain to add his thoughts to the mix.

> What if we make `P' always insert-enqueue-play in the library
> and play in the playlist (it's unbound now)?  Then one could
> use `% m ... RET P' to mark and then play.

Isn't it bound to `bongo-previous'?

>> +(defun bongo-line-marker ()
>> +  "Return a new marker for the current line."
>> +  (set-marker (make-marker) (bongo-point-at-bol)))
>> +
>> +(defun bongo-marked-track-line-p ()
>> +  "Return non-nil if the line at point is marked."
>> +  (member (bongo-line-marker) bongo-marks))
>
> This is wasteful, because you keep allocating new markers.
> It's also inefficient, because of the need to perform a
> linear search for every call to `bongo-marked-track-line-p'.

Right. Now, this was only a quick hack for you to test the
functionality, but if it wasn't for the redisplay function I'd say it
didn't matter much.

Another performance issue is that unused markers slow down Emacs. They
should be pointed "nowhere" after use.

I considered using text properties from the start instead, but I was
attached to the idea that the user could mark tracks in a certain
order. Also, remarking a track would pop the marker to the front of
the list. Using both properties and the list is good, though.

>> +(defun bongo-force-visible-line ()
>
> This shouldn't be necessary, of course, but maybe it's
> desirable to unhide newly-marked tracks anyway?

Yes, I think so. This is the least surprise to the user.

>> +(defun bongo-mark-lines-regexp (regexp &optional separate-fields)
>> +  "Mark lines with fields matching REGEXP.
>> +With prefix arg, match fields artist, album and track title
>> +separately.  Otherwise, match against a formatted infoset."
>
> Why is the `separate-fields' functionality needed?

It's not needed. I wrote it like that first, and added the default
functionality when I saw I was wrong. I left the old function
definition in like this so that you could try it out. (Yes, I should
have explained it in a comment.)