Add clear feature (Issue #40) #43
Conversation
lib/help.js
Outdated
| @@ -19,6 +19,7 @@ module.exports = ` | |||
| --priority, -p Update priority of task | |||
| --archive, -a Display archived items | |||
| --restore, -r Restore items from archive | |||
| --clean, -k Delete all checked items | |||
There was a problem hiding this comment.
I think it would be good to keep -cl or similar like that instead of -k.
There was a problem hiding this comment.
Sorry, It's my bad I forgot about that meow doesn't support the multi-character
There was a problem hiding this comment.
Do we want to keep the capital C alias or revert back to k?
There was a problem hiding this comment.
Keep as it is. Let's see what @klauscfhq thinks.
|
Updated alias to I did attempt to update it to |
|
@JoshuaCrocker The library used for aliases is meow, and it doesn't allow for multi-char aliases I guess, there is some sort of glitch or something,
See? also c flag alias has already used been used for check flag I guess so how can it be used for clean too? |
|
@SakhirAtwi The Clean command makes use of the capital |
cli.js
Outdated
| }, | ||
| clean: { | ||
| type: 'boolean', | ||
| alias: 'C' |
There was a problem hiding this comment.
Considering that this a console app and the fact that clear is a widely-used and familiar command among CLI users, with a closely similar effect to that of the one here, it (clear) seems to be a more appropriate flag name than clean. Also, let's completely avoid setting a shortcut/alias, in order to prevent any accidental uses.
clear: {
type: 'boolean'
}
index.js
Outdated
| @@ -57,6 +57,10 @@ const taskbookCLI = (input, flags) => { | |||
| return taskbook.moveBoards(input); | |||
| } | |||
|
|
|||
| if (flags.clean) { | |||
| return taskbook.clean(); | |||
| } | |||
There was a problem hiding this comment.
if (flags.clear) {
return taskbook.clear();
}
lib/help.js
Outdated
| @@ -19,6 +19,7 @@ module.exports = ` | |||
| --priority, -p Update priority of task | |||
| --archive, -a Display archived items | |||
| --restore, -r Restore items from archive | |||
| --clean, -C Delete all checked items | |||
There was a problem hiding this comment.
--clear Delete all checked items
lib/help.js
Outdated
| @@ -39,4 +40,5 @@ module.exports = ` | |||
| $ tb --list pending coding | |||
| $ tb --archive | |||
| $ tb --restore 4 | |||
| $ tb --clean | |||
lib/taskbook.js
Outdated
| @@ -499,6 +499,18 @@ class Taskbook { | |||
| this._save(_data); | |||
| render.successPriority(id, level); | |||
| } | |||
|
|
|||
| clean() { | |||
lib/taskbook.js
Outdated
| @@ -499,6 +499,18 @@ class Taskbook { | |||
| this._save(_data); | |||
| render.successPriority(id, level); | |||
| } | |||
|
|
|||
| clean() { | |||
| const keysToDelete = []; | |||
There was a problem hiding this comment.
To make things simpler keysToDelete can be renamed to just ids and _data can be explicitly declared & pushed to the top:
const ids = [];
const {_data} = this;
lib/taskbook.js
Outdated
| if (item.isComplete) { | ||
| keysToDelete.push(id); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Object.entries can be replaced by Object.keys so the whole method is compatible with node.js v6:
Object.keys(_data).forEach(id => {
if (_data[id].isComplete) {
ids.push(id);
}
});
lib/taskbook.js
Outdated
| } | ||
| }); | ||
|
|
||
| this.deleteItems(keysToDelete); |
There was a problem hiding this comment.
Before calling this.deleteItems(ids) we should check if the ids array is actually empty and return if so:
if (ids.length === 0) {
return;
}
this.deleteItems(ids);
|
Thank you a lot for taking the time to contribute : ) |
klaudiosinani
changed the title
Adds in functionality defined in #40, to delete all checked tasks by running a command.
This has been implemented in the
tb --cleancommand, which will iterate through all tasks and will pass any checked tasks to the standard delete method.Closes #40 @tmyers273