Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting Alfred variables with alfy.output (#43) #44

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,51 @@ alfy.alfred = {

alfy.input = process.argv[2];

alfy.output = arr => {
console.log(JSON.stringify({items: arr}, null, '\t'));
const isDefined = x => x !== null && x !== undefined;

const wrapArg = item => {
const alfredworkflow = {arg: item.arg, variables: item.variables};
const arg = JSON.stringify({alfredworkflow});
const newItem = Object.assign({}, item, {arg});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just put argument on the item object directly. No need to copy it I guess.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your reasoning. But I copied the item because I wanted to avoid this kind of scenario:

const unicorn = {
  title: 'unicorn',
  arg: '🦄',
  variables: {
    selectedMagicalBeing: true
  }
};

alfy.output([unicorn]);

const unicornEmoji = unicorn.arg;
doSomething(unicornEmoji);
// weird behavior. unicornEmoji is now a stringified JSON object

Obviously, you can avoid this sort of problem by calling alfy.output last. And I'm sure that's how most people use it. I just don't want Alfy users to have to remember to call alfy.output last.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General rule is to always use Object.assign if you modify a user-specified object. Not doing so can cause subtle bugs.

delete newItem.variables;
return newItem;
};

const formatMods = item => {
const copy = Object.assign({}, item);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to copy it first. It doesn't really matter in this case if we alter the object or not.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Object.keys(item.mods).forEach(modKey => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are more fan of for...of :)

for (const mod of Object.keys(item.mods)) {
    item.mods[mod] = wrapArg(item.mods[mod]);
}

return item;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! That syntax looks a little nicer to me too. I was wondering myself whether to go with the loop body you suggested.

Done 😄

const modItem = item.mods[modKey];
copy.mods[modKey] = wrapArg(modItem);
});

return copy;
};

// see https://www.alfredforum.com/topic/9070-how-to-workflowenvironment-variables/
// for documentation on setting environment variables from Alfred workflows
const format = item => {
if (!isDefined(item)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if item is not defined, we should make it an empty object instead.

item = item || {};

And just continue. This will make sure that we don't output null or undefined to Alfred which will probably result in a crash.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried outputting an array of nulls and noticed something pretty interesting. If you're calling your script from a script filter, Alfred just hangs when you output an array of non-objects.

// Example 1: Alfred hangs when this is called from a script filter
alfy.output(['hi', 'hey']);

// Example 2: same for this
alfy.output([null, null, undefined]);

When called from a Run Script action, you just get stringified JSON as your Alfred {query}:

'{
  "items": [
    null,
    null
  ]
}'

How does that influence our decision here? Not quite sure. If we use

item = item || {};

that will protect users from example 2 above, but not from example 1. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just throw if it's a non-object. Better to fail loudly than being silently "smart".

Could use: https://github.com/sindresorhus/is-plain-obj

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return item;
}

if (isDefined(item.variables)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just write if (item.variables) {

item = wrapArg(item);
}

if (isDefined(item.mods)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (item.mods) {

item = formatMods(item);
}

return item;
};

alfy.output = items => {
if (Array.isArray(items)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To come back to the throw new TypeError() thing. Just wondering, is there a reason why you would use alfy.output without providing an Array? If not, I think I like to be more explicit about it and throw an error instead.

// @sindresorhus

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a little more research into this question, and I agree with you about throwing the TypeError, @SamVerschueren. I was misremembering the previous behavior of alfy.output. It already assumes you've passed an Array:

// previous implementation
alfy.output = arr => {
  console.log(JSON.stringify({items: arr}, null, '\t'));
};

It's true that the previous implementation wouldn't throw a TypeError, but I think the error is actually better. It prevents people from doing stuff like

alfy.output('hi!');
// outputs '{"items":"hi!"}'

which doesn't make sense.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @sindresorhus agrees, let's move forward with throwing a TypeError.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, should throw a TypeError.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

items = items.map(format);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use items.map(item => format(item)). The reason for this is that the map callback returns multiple parameters. This results in sending all those parameters to format which may result in bugs in the future if for instance format starts excepting additional input arguments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't think about that! Good point. Fixed 😄

}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use items.map

items = items.map(item => format(item));

And then return the item in the format function.

console.log(JSON.stringify({items}, null, '\t'));
};

alfy.matches = (input, list, item) => {
Expand Down
Binary file added media/screenshot-variable.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 35 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ Return output to Alfred.

Type: `Array`

List of `Object` with any of the [supported properties](https://www.alfredapp.com/help/workflows/inputs/script-filter/json/).
List of `Object` with any of the [supported properties](https://www.alfredapp.com/help/workflows/inputs/script-filter/json/). In addition, if a list item has a `variables` property, it will be used to set Alfred's [Workflow Environment Variables](https://www.alfredapp.com/help/workflows/advanced/variables/) if the user selects the item. More info on environment variables is [here](https://www.alfredforum.com/topic/9070-how-to-workflowenvironment-variables/).

Example:

Expand All @@ -179,6 +179,40 @@ alfy.output([{

<img src="media/screenshot-output.png" width="694">

Using the `variables` property:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should put this under it's own section so we can easily link it from the highlights and to make it more clear. Not sure where though, any ideas @sindresorhus? Maybe a new section Environment variables under Usage or Example? Or on the same level as those?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new top-level section called "Environment variables".


```js
alfy.output([
{
title: 'Unicorn',
arg: '🦄',
variables: {
color: 'white'
}
},
{
title: 'Rainbow',
arg: '🌈',
variables: {
color: 'myriad'
}
}
]);
```

You can access Alfred Workflow Variables through `process.env`:

```js
// After a user selects "Unicorn" or "Rainbow"
process.env.color
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we alias this as alfy.env?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be nice.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

//=> 'white' if they selected Unicorn
//=> 'myriad' if they selected Rainbow
```

Alfred Workflow Variables are also available in the workflow editor using the form `{var:varname}`. For example, `{var:color}`

<img src="media/screenshot-variable.png" width="694">

#### matches(input, list, [item])

Returns an `Array` of items in `list` that case-insensitively contains `input`.
Expand Down
128 changes: 128 additions & 0 deletions test/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import {serial as test} from 'ava';
import hookStd from 'hook-std';
import {alfy} from './_utils';

const m = alfy();
const hook = cb => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return a promise instead of a callback, this way you can just use async/await in your tests.

const unhook = hookStd.stdout({silent: true}, output => {
unhook();
if (cb) {
cb(output);
}
});
};

test.cb('.output() properly wraps item.variables', t => {
hook(output => {
const item = JSON.parse(output).items[0];
const arg = JSON.parse(item.arg);
t.deepEqual(arg, {
alfredworkflow: {
arg: '🦄',
variables: {fabulous: true}
}
});
t.end();
});
m.output([{
title: 'unicorn',
arg: '🦄',
variables: {fabulous: true}
}]);
});

test.cb('.output() wraps item.variables even if item.arg is not defined', t => {
hook(output => {
const item = JSON.parse(output).items[0];
const arg = JSON.parse(item.arg);
t.deepEqual(arg, {
alfredworkflow: {
variables: {fabulous: true}
}
});
t.end();
});
m.output([{
title: 'unicorn',
variables: {fabulous: true}
}]);
});

test.cb('.output() does not wrap item.arg if item.variables is not defined', t => {
hook(output => {
const item = JSON.parse(output).items[0];
t.is(item.arg, '🦄');
t.is(item.variables, undefined);
t.end();
});
m.output([{
title: 'unicorn',
arg: '🦄'
}]);
});

test('.output() accepts null and undefined items', t => {
hook();
t.notThrows(() => m.output([undefined, null]));
});

test('.output() accepts non-arrays', t => {
let done = false;
const unhook = hookStd.stdout({silent: true}, () => {
if (done) {
unhook();
}
});
t.notThrows(() => m.output({}));
t.notThrows(() => m.output('unicorn'));
t.notThrows(() => m.output(null));
done = true;
t.notThrows(() => m.output(undefined));
});

const itemWithMod = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this at the top.

title: 'unicorn',
arg: '🦄',
variables: {fabulous: true},
mods: {
alt: {
title: 'Rainbow',
arg: '🌈',
variables: {
color: 'myriad'
}
}
}
};

test.cb('.output() wraps mod items', t => {
hook(output => {
const item = JSON.parse(output).items[0];
const altArg = JSON.parse(item.mods.alt.arg);
t.deepEqual(altArg, {
alfredworkflow: {
arg: '🌈',
variables: {
color: 'myriad'
}
}
});
t.end();
});
m.output([itemWithMod]);
});

test.cb('.output() doesn\'t change original item when mod items are present', t => {
hook(output => {
const item = JSON.parse(output).items[0];
const arg = JSON.parse(item.arg);
t.deepEqual(arg, {
alfredworkflow: {
arg: '🦄',
variables: {fabulous: true}
}
});
t.end();
});
m.output([itemWithMod]);
});