From 6fea30c21dc4642ca1043a45f8f87a9cef5726d4 Mon Sep 17 00:00:00 2001 From: Florrie Date: Wed, 20 Sep 2017 13:19:14 -0300 Subject: Show path to track's parent in track info - not including the track, which is redundant --- src/loop-play.js | 9 ++-- src/playlist-utils.js | 145 +++++++++++++++++++++++++++++++++++++------------- todo.txt | 6 ++- 3 files changed, 120 insertions(+), 40 deletions(-) diff --git a/src/loop-play.js b/src/loop-play.js index 810d1c6..5d0048b 100644 --- a/src/loop-play.js +++ b/src/loop-play.js @@ -13,13 +13,16 @@ const FIFO = require('fifo-js') const EventEmitter = require('events') const promisifyProcess = require('./promisify-process') const killProcess = require('./kill-process') -const { getItemPathString, safeUnlink } = require('./playlist-utils') const { HistoryController, generalPicker } = require('./pickers') const { getDownloaderFor, byName: downloadersByName, makeConverter } = require('./downloaders') +const { + getItemPathString, safeUnlink, parentSymbol +} = require('./playlist-utils') + class Player { constructor() { this.disablePlaybackStatus = false @@ -499,7 +502,7 @@ class PlayController extends EventEmitter { const getColorMessage = t => { if (!t) return '\x1b[2m(No track)\x1b[0m' - const path = getItemPathString(t) + const path = getItemPathString(t[parentSymbol]) return ( `\x1b[1m${t.name} \x1b[0m@ ${path} \x1b[2m(${t.downloaderArg})\x1b[0m` @@ -509,7 +512,7 @@ class PlayController extends EventEmitter { const getCleanMessage = t => { if (!t) return '(No track)' - const path = getItemPathString(t) + const path = getItemPathString(t[parentSymbol]) return `${t.name} @ ${path}` } diff --git a/src/playlist-utils.js b/src/playlist-utils.js index 2e6bbb9..5c8f206 100644 --- a/src/playlist-utils.js +++ b/src/playlist-utils.js @@ -150,7 +150,12 @@ function filterPlaylistByPathString(playlist, pathString) { function filterGrouplikeByPath(grouplike, pathParts) { // Finds a group by following the given group path and returns it. If the // function encounters an item in the group path that is not found, it logs - // a warning message and returns the group found up to that point. + // a warning message and returns the group found up to that point. If the + // pathParts array is empty, it returns the group given to the function. + + if (pathParts.length === 0) { + return grouplike + } const titleMatch = (group, caseInsensitive = false) => { let a = group.name @@ -194,15 +199,26 @@ function removeGroupByPath(playlist, pathParts) { const groupToRemove = filterGrouplikeByPath(playlist, pathParts) - const parentPath = pathParts.slice(0, pathParts.length - 1) - let parent + if (playlist === groupToRemove) { + console.error( + 'You can\'t remove the playlist from itself! Instead, try --clear' + + ' (shorthand -c).' + ) - if (parentPath.length === 0) { - parent = playlist - } else { - parent = filterGrouplikeByPath(playlist, parentPath) + return } + if (!(parentSymbol in groupToRemove)) { + console.error( + `Group ${pathParts.join('/')} doesn't have a parent, so we can't` + + ' remove it from the playlist.' + ) + + return + } + + const parent = groupToRemove[parentSymbol] + const index = parent.items.indexOf(groupToRemove) if (index >= 0) { @@ -210,7 +226,7 @@ function removeGroupByPath(playlist, pathParts) { } else { console.error( `Group ${pathParts.join('/')} doesn't exist, so we can't explicitly ` + - "ignore it." + 'ignore it.' ) } } @@ -271,23 +287,23 @@ function getItemPathString(item) { // Requires that the given item be from a playlist processed by // updateGroupFormat. - const displayName = item.name || '(Unnamed)' - + // Check if the parent is not the top level group. + // The top-level group is included in the return path as '/'. if (item[parentSymbol]) { - // Check if the parent is not the top level group. - // The top-level group is never included in the output path. + const displayName = item.name || '(Unnamed)' + if (item[parentSymbol][parentSymbol]) { return getItemPathString(item[parentSymbol]) + '/' + displayName } else { - return displayName + return '/' + displayName } } else { - return displayName + return '/' } } function parsePathString(pathString) { - const pathParts = pathString.split('/') + const pathParts = pathString.split('/').filter(item => item.length) return pathParts } @@ -352,10 +368,47 @@ module.exports = { } if (require.main === module) { + const compareArrays = (a, b) => { + return a.length === b.length && a.every((x, i) => b[i] === x) + } + + const _assert = (value, condition) => { + if (condition(value)) { + console.log(' ..good.') + } else { + console.log(' BAD! result:', value) + } + } + + const assert = (value, expectedValue) => { + return _assert(value, x => x === expectedValue) + } + + const assertArray = (value, expectedValue) => { + return _assert(value, x => compareArrays(x, expectedValue)) + } + + console.log('compareArrays') + + { + console.log('- ([a, b], [a, b]) should return true') + assert(compareArrays(['a', 'b'], ['a', 'b']), true) + } + + { + console.log('- ([a, b], [30, 20]) should return false') + assert(compareArrays(['a', 'b'], [30, 20]), false) + } + + { + console.log('- ([a, b], [a, b, c]) should return false') + assert(compareArrays(['a', 'b'], ['a', 'b', 'c']), false) + } + console.log('getItemPathString') { - console.log('- (root with name) should output a/b/c/Foo') + console.log('- (root with name) should return /a/b/c/Foo') const playlist = updatePlaylistFormat( {name: 'root', items: [ @@ -371,16 +424,11 @@ if (require.main === module) { const deepTrack = playlist.items[0].items[0].items[0].items[0] - const path = getItemPathString(deepTrack) - if (path === 'a/b/c/Foo') { - console.log(' ..good.') - } else { - console.log(' ..BAD! result:', path) - } + assert(getItemPathString(deepTrack), '/a/b/c/Foo') } { - console.log('- (root without name) should output a/b/c/Foo') + console.log('- (root without name) should return /a/b/c/Foo') const playlist = updatePlaylistFormat( {items: [ @@ -396,16 +444,11 @@ if (require.main === module) { const deepTrack = playlist.items[0].items[0].items[0].items[0] - const path = getItemPathString(deepTrack) - if (path === 'a/b/c/Foo') { - console.log(' ..good.') - } else { - console.log(' ..BAD! result:', path) - } + assert(getItemPathString(deepTrack), '/a/b/c/Foo') } { - console.log('- (sub-group without name) should output a/b/(Unnamed)/c/Foo') + console.log('- (sub-group without name) should return /a/b/(Unnamed)/c/Foo') const playlist = updatePlaylistFormat( {items: [ @@ -423,11 +466,41 @@ if (require.main === module) { const deepTrack = playlist.items[0].items[0].items[0].items[0].items[0] - const path = getItemPathString(deepTrack) - if (path === 'a/b/(Unnamed)/c/Foo') { - console.log(' ..good.') - } else { - console.log(' ..BAD! result:', path) - } + assert(getItemPathString(deepTrack), '/a/b/(Unnamed)/c/Foo') + } + + { + console.log('- (path string of root) should return /') + + const playlist = updatePlaylistFormat({items: []}) + + assert(getItemPathString(playlist), '/') + } + + console.log('parsePathString') + + { + console.log('- (foo/bar/baz) should return [foo, bar, baz]') + assertArray(parsePathString('foo/bar/baz'), ['foo', 'bar', 'baz']) + } + + { + console.log('- (/foo/bar/baz) should return [foo, bar, baz]') + assertArray(parsePathString('/foo/bar/baz'), ['foo', 'bar', 'baz']) + } + + { + console.log('- (/) should return []') + assertArray(parsePathString('/'), []) + } + + { + console.log('- (/////foo) should return [foo]') + assertArray(parsePathString('/////foo'), ['foo']) + } + + { + console.log('- (//foo/////bar//) should return [foo, bar]') + assertArray(parsePathString('//foo/////bar//'), ['foo', 'bar']) } } diff --git a/todo.txt b/todo.txt index 44caebb..a3e576c 100644 --- a/todo.txt +++ b/todo.txt @@ -357,9 +357,13 @@ TODO: Make the '@ ...' part of track-info show the path to the track, rather (Done!) TODO: Only show the path to a given track's group in the track info. + (Done!) TODO: Figure out what to show in the '@ ...' part when a track is in the top - level (i.e. its group is only the top). Probably just '/'? + level (i.e. its group is only the top). Probably just '/'? Probably best + to make sure that the to-path-string function always returns a slash at + the start. + (Done!) TODO: Let track objects have an option for command line arguments to pass to the converter program (ffmpeg or avconv). This would be useful for a -- cgit 1.3.0-6-gf8a5