Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

Rebased variables on WFSerialization. Added variable's with method #47

Merged
merged 7 commits into from
Dec 12, 2018

Conversation

xAlien95
Copy link
Contributor

@xAlien95 xAlien95 commented Dec 4, 2018

Checks

  • I've checked there are no linting errors.
  • I've checked the code is still at 100% test coverage.

Added Actions (if relevant)

  • Date
  • Format Date

Are you happy to be listed as a contributor on Shortcuts.fun?

Yes

Any other information / comments

  • 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 (Decide on rules for naming Actions #26)
  • Added TypeDoc documentation for .with() method, with some examples

Contributors that have to implement actions that accept variables as parameters only need to 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:

Imgur

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

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

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

@xAlien95
Copy link
Contributor Author

xAlien95 commented Dec 4, 2018

This pull request is still in WIP.

AggrandizementPropertyUserInfo, AggrandizementPropertyName and WFDateFormatStyle aren't complete. I'll fill WFDateFormatStyle in the next days and I'll add date() and formatDate() actions.

The .with() method will get full capabilities in the next (and last?) pull request (step 3 of this rebase process #14). At the moment it can only set the coercion type and get values for Dictionary keys. This pull request is mainly here for the variable rebase from Attachment to WFSerialization type.

As always, suggestions are welcome 🙂

@xAlien95
Copy link
Contributor Author

xAlien95 commented Dec 6, 2018

Date formatting aggrandizements are now fully implemented.

@joshfarrant, I'm removing the "To do" checks since they require further discussion.
The checks were the following:

  • Update actions that can handle variables as parameter to have WFSerialization as additional parameter type

This one probably requires edits in the CONTRIBUTING guide, since all the current and future actions should have to implement tests for cases where askWhenRun, magic variables and variables are used in parameters. A separate pull request may be a better place to add this, since a list of checks could be added to track the "currently implemented action updating" process.

  • Update utils/buildSerialization.ts to handle withVariables tagged template strings

The actual buildSerialization is based on simplicity and it let the users to just type a JS object. That's great, but it has a few limitations: it can't have withVariables keys, it can't explicitly specify a value's type, it cannot be reversed (i.e. it's not suitable for the .shortcut to .js parser). I'm going to open an issue about it and about possible alternatives.

This pull request is now complete. The aggrandizement system now misses only the get parameter, I'll add it in the third and last step of this process since it requires more work: the only way to implement it correctly and thoroughly is by creating 251 aggrandized variables with all the combinations I listed here and by parsing each one of them to get the right PropertyUserInfo attribute.

This pull request can be now reviewed and, if there won't be suggestions or issues, merged. 🙂

@joshfarrant
Copy link
Owner

Thanks @xAlien95, I’ll take a proper look at this over the weekend.

@joshfarrant
Copy link
Owner

🎉 💙

@joshfarrant joshfarrant merged commit 0790014 into joshfarrant:master Dec 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants