feat: Allow additional tasks states and filtering by states #675 #670

sytone · 2022-05-19T18:34:58Z

Description

When toggling a task, it should be able to move through more states than Todo (- [ ]) or Done (- [x]). This new feature will allow users to provide additional states with - [-] and - [/] being available by default.

The users will also be able to add their own status types and transistions between them to allow further customization.

Changes

Add new script to generate a development build with sourcemap included for debugging in Obsidian.
Add / Update tests for new Status types.
Update settings UI to allow users to add custom types.
Update the Task UI to allow user to select new status types.
Add / Update query to handle any Status changes and update unit tests.
Update Sort to handle new status options.

Motivation and Context

There have been multiple asks to have additional staus types in the discussions and other plugins now support this. To ensure the Tasks plugin can support all these requests and features the new status model has been created.

How has this been tested?

This has been tested via the new unit tests for the status and status registry. This will cover the existing functionality as well as the new ability to add new status types and transisii0ons. Currently there is no automated UI testing. That is being done manually.

As this change also impacts the Task object, UI and settings all related tests have been validated and updated as needed.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My code follows the code style of this project and passes yarn run lint.
My change requires a change to the documentation.
I have updated the documentation accordingly.
My change has adequate Unit Test coverage.

By creating a Pull Request you agree to our Code of Conduct. For further guidance on contributing please see contributing guide

Add dev setup to contributing Add tags as a property of task Add query by tag in query language Add sort by tag Added unit tests for coverage

Implement feedback on filter and tag sorting, ready for next release obsidian-tasks-group#632

…case insensative

docs: add documentation on tag sorting fix: handle lack of tag in the comparison for tag sorting test: add increased test coverage to tags

Fixes obsidian-tasks-group#666

sytone · 2022-05-19T18:41:12Z

@schemar & @claremacrae here is the draft of adding new status types, will make the task management and tracking more flexible. I am using it locally and working fine.

…s import

A task with a character between the square brackets should be marked as checked and have the is-checked class on the parent LI

Allows optional or features in development to be enabled conditionally, there is a second commit coming to move the new functionality under the relevant flags which will make the PR review and check in simpler as code can go in but not be used until the flag is set.

schemar · 2022-05-22T10:37:39Z

Thank you, @sytone. It appears this PR includs changes that are not relevant to this PR, or am I seeing that wrong? Does this PR depend on earlier PRs?

sytone · 2022-05-22T22:57:27Z

It was only dependent on the tags pr. I have added the features capabilities as I am planning on adding the menu option which will be in a preview based mode to see how other users would want to use it.

What extra did you see.

sytone · 2022-05-22T23:06:39Z

oh it is the append global filter. I meant to push that to a separate branch on top of this and forgot to run command. Will pull it out

…ssue666

claremacrae

Many thanks for doing this. I know the functionality will be greatly appreciated.

Some initial comments.

claremacrae · 2022-05-23T07:44:25Z

esbuild.config.mjs

-
-const banner =
-`/*
+import process from 'process';


This file has been reformatted, which makes it impossible to see if or where the content has changed.

And also, not possible to know whether any change was related to the topic of this PR, or something independent that could/should have been in a separate PR.

Ill reset the structure, ironically this is now formatted using the same settings as lint 😄

claremacrae · 2022-05-23T07:48:56Z

.gitattributes

@@ -0,0 +1,2 @@
+*.js text eol=lf


How does changing line endings relate to "feat: Allow additional tasks states and filtering by states"?

Could this be a separate PR, with a statement about motivation?

What impact will this have on anyone who already has a clone of this repo on Windows? In the past, when these attributes have changed, I think I have struggled to get a clean repo, because it sees any local CRLF as differences. I seem to recall being confused, in any case.

From my own experience, I feel that mixing (seemingly) unrelated changes inside one PR generally makes it harder for reviews. The more focussed a PR is, the quicker it can be reviewed, and the quicker separate PRs for later features can be started.

This reduces noise in windows, if you have CRLF and then add them in the file will just go away as git is smart enough to know that they are the same on commit/push. However in a IDE they will be different as CRLF <> LF and you will get reg conflicts.

Adding this in just lets the IDE/git know they should always be LF on all operating systems and to ignore the git config settings for LF/CRLF for these files.

I can add this to a very small PR if needed, typically we would not make a separate PR for something like this and place it in a change going in as it is small and easily identified.

Made #682, so when it goes in an I merge with main this file will go away from this PR.

claremacrae · 2022-05-23T07:51:24Z

package.json

    "lint": "eslint ./src --fix && eslint ./tests --fix && tsc --noEmit --pretty && svelte-check",
    "test": "jest --ci",
-    "test:dev": "jest --watch"
+    "test:dev": "jest --watch",
+    "test:obsidian": "pwsh -ExecutionPolicy Unrestricted -NoProfile -File ./scripts/Test-TasksInLocalObsidian.ps1"


Ooh, using PowerShell to drive Obsidian. This is very intriguing!

Will it work on Linux and Mac? (I know that PowerShell has been released on those platforms, but I don't know whether it can drive Obsidian on those platforms.

Later: it doesn't drive obsidian, it copies a distribution in to a vault.

Yes, it works across platforms. I use it on my linux machine and in WSL on windows. I am using it as bash sends me around the twist these days with string manipulation. Being able to work with objects in the shell is much better.

I am slowly playing with a PowerShell module that will work with obsidian and the shell plugin also supports running PowerShell scripts. This will setup synlinks to the vault you specify in a environment variable or via the command line and then start the build watch process. Makes the dev cycle faster.

claremacrae · 2022-05-23T07:53:54Z

scripts/Test-TasksInLocalObsidian.ps1

@@ -0,0 +1,53 @@
+[CmdletBinding()]


Oh, based on the file name, I misunderstood the purpose of this file. I though it was actually testing Obsidian, but it is copying the built plugin in to an Obsidian vault.

That's still very useful, but also a candidate for being in a separate PR please (alongside the associated change to package.json

I am using it to help me work on this PR. So there will be a chicken egg scenario. Ill make a separate PR and if it goes in first then this will disappear from this PR.

claremacrae · 2022-05-23T07:55:04Z

src/Commands/CreateOrEdit.ts

@@ -64,11 +66,10 @@ const taskFromLine = ({ line, path }: { line: string; path: string }): Task => {
        // Should never happen; everything in the regex is optional.
        console.error('Tasks: Cannot create task on line:', line);
        return new Task({
-            status: Status.Todo,
+            status: Status.TODO,


For my own education, I'm curious as to why Todo -> TODO?

Asking as it increases the number of changes to review, not because I want to change it.

Consistency in code, as these are constants I have seen they should be upper case in TS/JS. Also makes it super easy to see when reading code that these are not a normal variable.

claremacrae · 2022-05-23T08:12:28Z

src/Feature.ts

+     * Compare to the fromString() method, which deserializes the object.
+     */
+    public toJSON() {
+        return this.internalName;


What is internalName?

It is the string name or key that is used to identify the feature. This makes sense now, seeing how Feature is used in the settings code.

Yes, it is the key, separate to the nicer one that users see.

claremacrae · 2022-05-23T08:15:02Z

src/Settings.ts

@@ -1,18 +1,31 @@
+import { Feature, FeatureFlag } from './Feature';


Seeing the feature flag code mixed in here makes me wonder if it would be possible for the feature flag feature to be pulled out and implemented as its own PR first?

I realised it may be too late for that now, but them being combined does mean a full review is going to take a while.

I could, see how much of a issue it is.

claremacrae · 2022-05-23T08:16:19Z

src/Settings.ts

 };

 let settings: Settings = { ...defaultSettings };

 export const getSettings = (): Settings => {
+    // Check to see if there is a new flag and if so add it to the users settings.


Does this mean that if this code is released, users who look at their settings will see the feature flag and be inclined to turn it on, not realising that the feature is not fully tested, and so initially disabled by default for a good reason?

I think that's probably fine, but it would still be helpful to understand....

It is in the settings at the bottom with indicators to its stability.

claremacrae · 2022-05-23T08:18:13Z

src/Settings.ts

@@ -21,3 +34,15 @@ export const updateSettings = (newSettings: Partial<Settings>): Settings => {

    return getSettings();
 };
+
+export const isFeatureEnabled = (internalName: string): boolean => {


Oh interesting - I had looked for this kind of convenience method in the Feature code, not realising it would be in the Settings code.

Totally happy for it to be here... If there isn't a comment in the Feature code mentioning these accessors, it would probably be worth adding one.

Will updated comments, the settings is responsible for what is on and off, feature just lets the plugin in know about a feature and its internal state.

claremacrae · 2022-05-23T08:23:02Z

tests/Query.test.ts

+    const defaultTasksWithStatus = [
+        '- [ ] Something to do',
+        '- [/] Something I am doing',
+        '- [x] Something I have done',
+        '- [-] Something I will no longer do',
+    ];


What's the thinking behind changing the default statuses?
Will there be any change in behaviour from the current?
Oh, this is test code. I was confused by the variable name...

… sytone/issue666

claremacrae · 2022-06-26T13:44:30Z

Closing. As well as containing unrelated changes, this is superseded by #799.

cmprmsd · 2022-12-23T23:31:14Z

In #2 (comment) @sytone proposed some changes that made it possible to have more than two state values and toggle them with clicks.

Was this pull request that change? I think after all the closed PRs the user got demotivated to further tackle this feature merge 😰

It was a great idea though!

rinogo · 2023-01-20T14:19:46Z

Bummer this was closed. It's exactly what I'm looking for. Maybe I'll create a standalone plugin to accomplish this.

claremacrae · 2023-01-20T15:04:05Z

Yes, this PR was closed for good reasons at the time.

BUT I have been spending 3 weeks working approximately full time on incorporating the code from this PR in to the latest Tasks code base, fixing bugs and usability and backwards compatibility issues.

I still have lots of work to do on the documentation.

Then it will be released soon.

Feel free to create another plugin if you wish @rinogo - but to get it releasable quality you will find it is a lot of work.

rinogo · 2023-01-20T15:06:26Z

That's great news! I was actually referring to specifically being able to cycle through each valid state (i.e. more than just unchecked/checked) of a checkbox onclick without any extra functionality. (which is a much more straightforward use case)

Certainly, it's a lot more work incorporating changes like this into a larger codebase!

Need any help testing? Happy to help out.

claremacrae · 2023-01-20T15:12:46Z

Need any help testing? Happy to help out.

Feel free: #1530

Do read the last two comments before spending ages setting up lots of custom statuses.

claremacrae · 2023-02-02T20:19:24Z

A heavily modified, fixed and documented version of this PR was released in Tasks 1.24.0 - with major credit given to the author of this PR both in the docs and the announcement.

sytone added 18 commits May 2, 2022 23:36

Markdown lint fixes

53d2d8e

Add dev setup to contributing Add tags as a property of task Add query by tag in query language Add sort by tag Added unit tests for coverage

Merge branch 'main' of https://github.com/schemar/obsidian-tasks

bea862f

feat: disregard the global tag

f2104c9

Implement feedback on filter and tag sorting, ready for next release obsidian-tasks-group#632

feat: allow tag query to be hashless

1bb8f9f

Merge branch 'main' of https://github.com/schemar/obsidian-tasks

d8a3168

test: refactor tag tests out and make them data driven

75eba71

feat: update query language, add tests, make hash optional and query …

d8a95b0

…case insensative

test: add tests to validate substring

547f776

test: chect to see if global tag is not found in query

9716007

docs: update filter documentation on the tag query

a3964f7

feat: add ability to sort by tag instance/index

9c64527

docs: add documentation on tag sorting fix: handle lack of tag in the comparison for tag sorting test: add increased test coverage to tags

Merge branch 'main' of https://github.com/schemar/obsidian-tasks

f90abc4

fix: resolved issues raised in PR.

4b88b4a

build: add markdown lint to list command

f02f2d2

fix: resolve remaining comments and revert markdown lint changes

67e97c6

docs: update grammar based issue and sorting description for tags

b2ace95

Merge branch 'main' of https://github.com/schemar/obsidian-tasks

3848523

feat: Allow additional tasks states and filtering by states

788e25e

Fixes obsidian-tasks-group#666

sytone added 7 commits May 19, 2022 20:01

feat: add cancelled and in progress along with minimal supported task…

de6cb0f

…s import

feat: add query capabilities for status

010f439

build: enable minify main.js 604.9kb -> 283.8kb

bd31de1

build: move script to symlinks

b08bdc8

fix: Update li rendering to match obsidian

f58186a

A task with a character between the square brackets should be marked as checked and have the is-checked class on the parent LI

update flags for next menu feature only

149eb34

feat: add git attributes to force LF on windows

d5976ba

sytone changed the title ~~feat: Allow additional tasks states and filtering by states #666~~ feat: Allow additional tasks states and filtering by states #675 May 22, 2022

Merge commit 'ff1216477242401139fee2031b28ebec9422334c' into sytone/i…

292f684

…ssue666

sytone mentioned this pull request May 23, 2022

Enable preview feature to allow global filter to be at the end. #678

Closed

7 tasks

sytone marked this pull request as ready for review May 23, 2022 00:48

claremacrae reviewed May 23, 2022

View reviewed changes

sytone added 2 commits May 29, 2022 12:44

Merge from upstream main

69b8517

Merge branch 'main' of https://github.com/schemar/obsidian-tasks into…

2b92499

… sytone/issue666

claremacrae mentioned this pull request May 31, 2022

I suggest a few more "states" for tasks #703

Closed

claremacrae closed this Jun 26, 2022

		@@ -1,18 +1,31 @@
		import { Feature, FeatureFlag } from './Feature';

Uh oh!

feat: Allow additional tasks states and filtering by states #675 #670

feat: Allow additional tasks states and filtering by states #675 #670

Uh oh!

Conversation

sytone commented May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Motivation and Context

How has this been tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

sytone commented May 19, 2022

Uh oh!

schemar commented May 22, 2022

Uh oh!

sytone commented May 22, 2022

Uh oh!

sytone commented May 22, 2022

Uh oh!

claremacrae left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claremacrae commented Jun 26, 2022

Uh oh!

cmprmsd commented Dec 23, 2022

Uh oh!

sytone commented May 19, 2022 •

edited

Loading

rinogo commented Jan 20, 2023 •

edited

Loading