Proposals on variables

Question

Proposals on variables

Opened this issue 6 years ago · 17 comments

I'm in the process of presenting a fork in which variables (named variables, magic variables and global variables (Ask, Clipboard, CurrentDate and ExtensionInput)) are objects instead of strings. In my local repository, I wrote:

const ActionOutput = (name?: string): Attachment => ({
  OutputUUID: uuidv4(),
  OutputName: name,
  Type: 'ActionOutput',
});

const Variable = (name: string): Attachment => ({
  VariableName: name,
  Type: 'Variable',
});

const Ask: Attachment = { Type: 'Ask' };
const Clipboard: Attachment = { Type: 'Clipboard' };
const CurrentDate: Attachment = { Type: 'CurrentDate' }; // TODO date/time formats
const ExtensionInput: Attachment = { Type: 'ExtensionInput' };

Using lowerCamelCase, an ask global variable would conflict with the ask() action function, so I ended up distinguishing them using CapitalCamelCase for variable objects. It could make sense since in my fork ActionOutput and Variable acts like JS types/classes:

const {
  buildShortcut,
  withVariables,
  ActionOutput,
  Clipboard,
} = require('@joshfarrant/shortcuts-js');
const {
  calculate,
  comment,
  number,
  showResult
} = require('@joshfarrant/shortcuts-js/actions');

// We'll use this later to reference the output of a calculation
const calcVar = ActionOutput('My result');

// Define our list of actions
const actions = [
  comment({
    text: 'Hello, world!',
  }),
  number({
    number: 42,
  }),
  calculate({
    operand: 3,
    operation: '/',
  }, calcVar),
  showResult({
    /**
     * We can use the result of the calculation in this Shortcuts's input
     * by passing the string to the 'withVariables' tag function
     */
    text: withVariables`Total is ${calcVar}! ${Clipboard}`,
  }),
];

Unfortunately the current lint script accepts variable names only in lowerCamelCase or UPPER_CASE.
How should I name the Ask global variable?

Answer 1 · 2018-11-22T19:49:18.000Z

I've also been thinking this out in my head. For the naming of the global variables, remember that the contribution guide suggests the display name from the shortcuts app itself (in English). My approach is quite similar to yours, but with a couple differences as described below:

// Global Variables
const askWhenRun: WFSerialization = {
  Value: {
    Type: 'Ask',
  },
  WFSerializationType: 'WFTextTokenAttachment',
};
const clipboard: WFSerialization = {
  Value: {
    Type: 'Variable',
  },
  WFSerializationType: 'WFTextTokenAttachment',
};
const currentDate: WFSerialization = {
  Value: {
    Type: 'CurrentDate',
  },
  WFSerializationType: 'WFTextTokenAttachment',
};
const shortcutInput: WFSerialization = {
  Value: {
    Type: 'ExtensionInput',
  },
  WFSerializationType: 'WFTextTokenAttachment',
};

// Util methods
const /** or makeMagicVariable */ actionOutput = (name?: string): WFSerialization => ({
  Value: {
    ...(name && { OutputName: name }),
    OutputUUID: uuidv4(),
    Type: 'ActionOutput',
  },
  WFSerializationType: 'WFTextTokenAttachment',
});
const formatSerialization = (variable: WFSerialization | string): WFSerialization => {
  // Named Variable
  if (typeof variable === 'string') {
    return {
      Value: {
        Type: 'Variable',
        VariableName: name,
      },
      WFSerializationType: 'WFTextTokenAttachment',
    };
  }

  // Already Serialized
  return variable;
};
const withVariables = (
  strings: TemplateStringsArray,
  ...vars: string[]
): WFSerialization => ({
  WFSerializationType: 'WFTextTokenString',
  Value: {
    string: strings.join('\ufffc'), // Join strings with object replacement character
    attachmentsByRange: strings.reduce(
      (a, _, i, s) => {
        // ...

        return {
          ...a,
          [`{${lengthSoFar}, 1}`]: formatSerialization(vars[i]).Value,
        };
      },
      {},
    ),
  },
});

Here I pulled the formatSerialization function from getVariable as a util function and made adjustments.

What this allows us is that any variable can be passed into withVariables and it will use just the Value object as WFTextTokenString serialized objects, while also allowing some actions to be able to take in the WFSerialization variable directly as a parameter for actions that support WFTextTokenAttachments.

Later on we can improve formatSerialization to be able to handle parameterizing variables (e.g. date formats, or changing the what input type to parse the variable as.

I'm curious what you think of my approach and any questions you have just ask.

Answer 2 · 2018-11-22T20:21:21.000Z

For the naming of the global variables, remember that the contribution guide suggests the display name from the shortcuts app itself (in English).

askWhenRun and shortcutInput look way better.

... allowing some actions to be able to take in the WFSerialization variable directly as a parameter for actions that support WFTextTokenAttachments.

I see, boolean values in dictionaries currently support WFTextTokenAttachments.

const formatSerialization = (variable: WFSerialization | string): WFSerialization => {
  // Named Variable
  if (typeof variable === 'string') {
    return {
      Value: {
        Type: 'Variable',
        VariableName: name,
      },
      WFSerializationType: 'WFTextTokenAttachment',
    };
  }

  // Already Serialized
  return variable;
};

If formatSerialization won't be used anywhere else, could it be renamed to variable(name: string) to become the equivalent of actionOutput()/magicVariable() for named variables? This way users will have to always use those functions to initialize variables and magic variables:

const result = magicVariable()
const myResult = magicVariable('My Result')

const list = variable('List')

Answer 3 · 2018-11-22T20:38:24.000Z

In that scenario where we have variable(), do you think withVariables should still have the ability to take in strings (producing the named variable), or enforce that all named variables have to be initialized? I think I like the latter, as it locks things down, making sure the user has one intention. Similarly we can lock down setVariable/addToVariable to take in WFSerialization instead of string, and just have the actions strip off VariableName for the WFVariableName.

@joshfarrant What do you think?

Answer 4 · 2018-11-22T20:53:27.000Z

... we can lock down setVariable/addToVariable to take in WFSerialization instead of string, ...

That's exactly what I did. It will remove every check/test and it will make it easier to write the shortcut2js converter (#4) later on.

Using WFSerialization as base, the getVariable action just becomes:

const getVariable = (
  {
    variable,
  }: GetVariableOptions,
): WFWorkflowAction => ({
  WFWorkflowActionIdentifier: 'is.workflow.actions.getvariable',
  WFWorkflowActionParameters: {
    WFVariable: variable,
  },
});

Answer 5 · 2018-11-22T21:20:58.000Z

I think I like the latter, as it locks things down, making sure the user has one intention.

@Archez I'd tend to agree with you in leaning towards the latter. Having withVariables take in strings for named variables can end up in somewhat counterintuitive code.

withVariables`Testing ${'Some Variable'}`;

Could very easily be assumed to be equal to

withVariables`Testing Some Variable`;

Which is clearly is not.

Initialising the named variables beforehand, while it will add a bit more boilerplate, will make the code easier to understand, and the user's intentions clearer.

I really like the direction this is going in, definitely a big improvement over my initial implementation!

Answer 6 · 2018-11-22T21:25:23.000Z

That is exactly my thought. The boiler plating aligns more with a 'code' structure rather than a 'iOS Shortcut' structure and also paves the way for a more concrete .shortcut -> .js parser.
Can't wait to see the results @xAlien95!

Answer 7 · 2018-11-22T21:30:36.000Z

Fantastic! Me too @xAlien95! 🧞‍♂️

Answer 8 · 2018-11-22T23:20:55.000Z

I pushed it to my fork, you can see and try it there.

@joshfarrant, do I have to split /src/utils/variables.ts in multiple files?
variable() and actionOutput() can be considered as util functions so it would make sense to store them in two separate files, but how should the global variables constants be stored in your opinion?

@Archez, should I use WFSerialization instead of Attachment as in your approach?

Note: I really don't know why my real name appeared twice on the commit.. It's the first time I'm using Atom to push commits. Any suggestion on how to prevent this behavior is highly appreciated!

Answer 9 · 2018-11-23T04:53:33.000Z

If we do not use the WFSerialization approach, then any action that can take in a WFTextTokenAttachment (e.g. number), would have to be responsible for ensuring that the proper serialization step is taken. On the other hand, if we take the WFSerialization approach, then actions do not have to know the difference between a regular variable and a withVariables serialized object, allowing the actions code to be straight forward. I guess it would be a choice between handling it one (initialized as WFSerialization) or handle per basis (actions build the serialized object). I'm leaning towards WFSerialization, but I am open to objections. I can not imagine it being an issue either way, as user made scripts would be unchanged.

For your note: I just use VSCode (its great just like Atom), and haven't had strange issues like that using it's built in git tools.

Answer 10 · 2018-11-23T22:41:38.000Z

@Archez, I'm not familiar with TypeScript, but if we use the WFSerialization approach we may need to always specify that the .Value property is a SerializationValue. TypeScript automatically assumes it's a string although it can have multiple types.

I split ../interfaces/WF/WFSerialization.ts in two files and I updated withActionOutput.ts (rebased version of withUUID.ts) like this:

import SerializationValue from '../interfaces/WF/SerializationValue';
import WFSerialization from '../interfaces/WF/WFSerialization';
import WFWorkflowAction from '../interfaces/WF/WFWorkflowAction';

export const withActionOutput = <OptionsType>(
  actionBuilder: (options: OptionsType) => WFWorkflowAction,
) => (
  (
    options: OptionsType,
    output?: WFSerialization,
  ): WFWorkflowAction => {
    const action = actionBuilder(options);

    if (output) {
      const value = <SerializationValue>output.Value;
      action.WFWorkflowActionParameters.UUID = value.OutputUUID;
      if (value.OutputName) {
        action.WFWorkflowActionParameters.CustomOutputName = value.OutputName;
      }
    }
    return action;
  }
);

Is there a better way to achieve it?

Answer 11 · 2018-11-25T08:18:33.000Z

if we use the WFSerialization approach we may need to always specify that the .Value property is a SerializationValue. TypeScript automatically assumes it's a string although it can have multiple types.

That's strange, it shouldn't prefer one type over any other, as far as I know.

As I'm sure you've seen, the WFSerialization interface is defined here, and it's Value should expect a SerializationValue.

This is the first TypeScript project I've built from the ground up, so there may well be a better way of defining the WFSerialization interface. Don't know how much TS experience you have @Archez, but do you have any thoughts on this?

EDIT: I should also mention that I agree with @Archez in leaning towards WFSerialization over Attachment, if possible.

Answer 12 · 2018-11-25T08:35:12.000Z

do I have to split /src/utils/variables.ts in multiple files?
variable() and actionOutput() can be considered as util functions so it would make sense to store them in two separate files, but how should the global variables constants be stored in your opinion?

I'd say have variable and actionOutput in their own files.

I'm wondering whether it might make sense to have another directory, src/variables, to store everything related to variables. Or maybe we'd keep variable and actionOutput as named exports from the main src/index.ts (and keep them stored in src/utils), and just have askWhenRun, clipboard, etc defined in a single src/variables.ts file.

That way, users would have something like:

import {
  buildShortcut,
  withVariables,
  variable,
  actionOutput,
} from '@joshfarrant/shortcuts-js';

import {
  askWhenRun,
  clipboard,
} from '@joshfarrant/shortcuts-js/variables';

Thoughts?

Answer 13 · 2018-11-25T10:22:26.000Z

I'm wondering whether it might make sense to have another directory, src/variables, to store everything related to variables.

Later on, it could be useful to have a src/variables directory. I'm still doing researches on the Aggrandizements property and I'm starting to think that a Variable class with a .with() method could suit well:

const result = variable('Result');

const actions = [
  number({
    number: 1112223333,
  }),
  setVariable({
    variable: result,
  }),
  showResult({
    text: withVariables`Number is ${result}, phone number is ${result.with({
      type: 'PhoneNumber',
    })}!`,
  }),
];

... we'd keep variable and actionOutput as named exports from the main src/index.ts ...

I agree with having both variable and actionOutput to be available from '@joshfarrant/shortcuts-js'.

@joshfarrant, this issue is going off-topic from the original title. Should I rename it to something like "Proposals on variables", or close this one and open a new one?

Answer 14 · 2018-11-25T20:01:39.000Z

@xAlien95 .with() sounds interesting. I've not looked into the Aggrandizements property at all - is that essentially casting the type to a phone number then (in your example)?

I agree about renaming the issue, please go ahead 🙂

Answer 15 · 2018-11-25T20:36:53.000Z

I've not looked into the Aggrandizements property at all - is that essentially casting the type to a phone number then (in your example)?

@joshfarrant yes (although a phone number cast does nothing at all, it just returns the number as a string since the number type doesn't have properties).

If you set a Text action with "Today at 9:00 am" and then you insert its output magic variable to be considered as a Date, you'll get the same time string you get typing "Today at 9:00 am" in the Date action. Aggrandizements can be avoided and replaced by actions, but they are useful if you don't want to break the action flow with actions like Get Dictionary Value: you can just cast a magic variable to a Dictionary and you'll be able to write the Key you want to obtain.

Answer 16 · 2018-12-02T14:49:27.000Z

@joshfarrant, step 2 of the variable implementation is live in my fork's variable-class branch.

Rebased variables on WFSerialization
Added Variable class with a .with() method for Aggrandizements
Added first implementation of some Aggrandizement types
Added PascalCase variable/class name validation in the linter (#26)

To do:

Update actions that can handle variables as parameter to have WFSerialization as additional parameter type
Update utils/buildSerialization.ts to handle withVariables tagged template strings
Add TypeDoc documentation for .with() method, with some examples

Contributors that have to implement actions that accept variables as parameters have to just add WFSerialization as additional type for that particular parameter (#27 (comment)), whilst users have to simply insert the variable:

getDeviceDetails({
  detail: askWhenRun,
})

Regarding .with(), its parameters are just the same that Shortcuts provides.
I put the coercion type under the type parameter:

const myVar = variable('My Variable');

myVar.with({
  type: 'Dictionary',
  getValueForKey: 'My Key',
});

myVar.with({
  type: 'Date',
  dateFormat: 'Custom',
  customFormat: 'dd MMM at HH:mm',
});

myVar.with({
  type: 'Image',
  get: 'Album',
});

You can take a look at all the changes here: xAlien95@024d0ba.
I'm open to suggestions and considerations 😄
I'll add a pull request if there won't be issues regarding this implementation.

In step 3 I'll implement all the Aggrandizements, I already listed them all in this gist.
After that work, there shouldn't be other significative modifications to prevent us to start working on the .shortcut -> .js parser (#4).

Answer 17 · 2018-12-04T17:52:00.000Z

@xAlien95 Looks fantastic, great work. Can't wait to see the PR, feel free to submit whenever you're ready.

After that work, there shouldn't be other significative modifications to prevent us to start working on the .shortcut -> .js parser.

This is very exciting. Everything else is coming along well, so I might start taking a look into ways to approach that soon.