"valid": false is a bad choice for the default for wf.NewItem

[mattmc3]

When calling wf.NewItem(title), the Item struct omits the Valid property in the Alfred JSON feedback with a value of false, which is really unfortunate because:

1. In the example above, we did not specify a validity setting - if we had wanted one, a call to wf.NewItem(title).Valid(false) would be the way to accomplish this
2. When you don't specify valid: false in Alfred's item JSON, it assumes you wanted true when omitted. The behavior of this library results in the opposite of the Alfred default behavior.
3. The README and docs don't mention this explicitly, so this can be tricky to track down.
4. With valid=false instead of valid=true as the default, the behavior of your workflow is to halt on the go script filter step you created, which can be very confusing a difficult to debug (I know, b/c I just wasted a lot of time finding that bug myself).

I understand that the go struct is likely just defaulting the boolean value to false, but for the reasons I outlined, I recommend that the correct default behavior of this property is use the omitempty JSON marshaling option. I'm happy to submit a fix/PR if you agree that this default is both incorrect and unintuitive.

[deanishe]

I understand that the go struct is likely just defaulting the boolean value to false

This is the main reason. It's first and foremost a Go library, where booleans are generally expected to be false by default. Or more precisely, I expect most people would expect false to be the default.

I did consider defaulting to true, or defaulting to true if an Arg or any variables are set.

But currently, I am unconvinced that they are intuitive enough to justify the "magic".

the opposite of the Alfred default behavior.

The library has never tried to follow Alfred's default behaviour. It imposes its own logic because Alfred's default behaviour never made a whole lot of sense (turns out a long-standing bug was at fault for much of it).

which can be very confusing a difficult to debug

In what way? This also seems to stem from your incorrect assumption that items are valid by default.

I recommend that the correct default behavior of this property is use the omitempty JSON marshaling option

That's a non-starter. If you set omitempty on a boolean, it is not emit when false. As Alfred defaults to true, that would make it impossible to generate a non-valid item.

In any case, there's no reason to not emit a value for valid (autocomplete is the only field where omitted vs empty matters). It's really only a question of what Item.valid gets set to by default in Feedback.NewItem().

[mattmc3]

Maybe I have asked this wrong. It’s less that the default should be true than that the ability to omit it all together is more consistent with how Alfred works in other contexts. By more intuitive, I mean that if you have made Alfred workflows before without the library, not specififying a parameter in the Items structure (ie: calling .Valid()) and having it appear anyway is a weird behavior.

That's a non-starter. If you set omitempty on a boolean, it is not emit when false.

Not so if you make the bool a *bool, which is how I am suggesting you would allow a three value situation, true, false, and omitted. It’s a two line fix in the feedback.go file. I am still convinced that defaulting to omitted is the better way.

My main argument for this change is to consider the user experience for a new user to this library. A new user experiments with a call to wf.NewItem("Hello World") and their workflow seems to work, but is inexplicably stuck on that step and never advances when "Hello World" is selected. They switch back to a quick Alfred JSON result: {"items": [{ "title": "Hello World" }]} and it works perfectly. Figuring out that there's this other "magic" property called "valid" that gets inserted even though it was never specified, and its behavior when false (the default) will cause your workflow to get stuck and not advance seems like a bad user experience and different from the way you are used to interacting with Alfred. Finding that there's this .Valid(true) call that needs to be made feels really rough.

If you are still unconvinced that making it omitempty is the wrong path, I would at least recommend mentioning .Valid() in the README.md example. That at least would be enough to tip off someone new to this library to not make this mistake.

BTW - quick side note: Thank you so much for making this library! I wasn't certain I needed a library vs hand-coding the JSON responses was the way to go, but the way you wrap panics, and handle environment variables and caching saved me a ton of development time. Thanks!

[deanishe]

I am suggesting you would allow a three value situation, true, false, and omitted.

What's the point? Omitting valid has the same effect as setting it to true while making the generated JSON less clear and uglying up the code with pointers to bools.

My main argument for this change is to consider the user experience for a new user to this library.

Okay. You're the first new user to mention having this issue.

Perhaps any other reading users can chime in if they were caught out by the behaviour, too?

Figuring out that there's this other "magic" property called "valid" that gets inserted even though it was never specified

There's nothing "magic" about the property, and if you understand Alfred's JSON format already, you can see exactly what's going on by looking at the generated JSON in Alfred's debugger (which is why the library pretty-prints the JSON).

I would at least recommend mentioning .Valid() in the README.md example

I think that would be potentially misleading, seeing as that example wants an invalid item. Explicitly setting Valid(false) on it might imply the default is true.

It is correctly and explicitly used in all the real examples, which are mentioned immediately after the example code in the README.

I wasn't certain I needed a library vs hand-coding the JSON responses

Yeah, you don't really need a library for Alfred 3. The library is Alfred 2 vintage, and its old XML format was much trickier to generate.

the way you wrap panics, and handle environment variables and caching saved me a ton of development time

So, its main utility is to eliminate boilerplate and provide some APIs that make it easy to very quickly plug some data source or useful library into Alfred.