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

Decide on rules for naming Actions #26

Closed
joshfarrant opened this issue Nov 30, 2018 · 6 comments
Closed

Decide on rules for naming Actions #26

joshfarrant opened this issue Nov 30, 2018 · 6 comments
Labels
discussion Discussion of future features

Comments

@joshfarrant
Copy link
Owner

CONTRIBUTING.md currently states

The file (and action) you're creating should be the name of the action as it appears in the Shortcuts app, camelCased.

This works fine for the majority of actions, however it's not perfect when naming actions with upper case words in the name, eg SSH and URL.

We might want to consider tweaking the rule to allow for action names to keep upper-case strings as upper-case. The table below shows some examples of this.

Action Name Proposed Current
Get Contents of URL getContentsOfURL getContentsOfUrl
Get Current IP Address getCurrentIPAddress getCurrentIPAddress
Run Script Over SSH runScriptOverSSH runScriptOverSSH
URL URL url

@xAlien95 has pointed out:

It's in line with standard JavaScript functions/properties such as innerHTML

Does anyone foresee any issues with one solution over the other? If not, I'll probably go with the proposed change.

@joshfarrant joshfarrant added the discussion Discussion of future features label Nov 30, 2018
@xAlien95
Copy link
Contributor

Does anyone foresee any issues with one solution over the other? If not, I'll probably go with the proposed change.

The current lint script gives an error with variable and function names not in camelCase:

ERROR: 19:7  variable-name  variable name must be in lowerCamelCase or UPPER_CASE

If new actions with names on the line of "URL From Text" will be added to Shortcuts, the proposed change won't work without modifying the linter.

It has also to be noted that in standard JS, locally upper-cased strings aren't always respected for acronyms. As an example: we have XMLHttpRequest instead of XMLHTTPRequest.

@xAlien95
Copy link
Contributor

@joshfarrant, since #47 has been merged, now all the action function names can be replaced with the proposed ones. In that pull request I added PascalCased variable name validation in tslint.json (that was required for the private Variable class).

@joshfarrant
Copy link
Owner Author

@xAlien95 I spotted that in your PR and was wondering if it would do the trick. I'm happy with that then, I'll go through and make the adjustments now.

@joshfarrant
Copy link
Owner Author

The naming of showInItunesStore still makes me kind of sad to look at. Would showInITunesStore be better? And if so should runJavascriptOnWebPage be runJavaScriptOnWebPage?

@xAlien95
Copy link
Contributor

The naming of showInItunesStore still makes me kind of sad to look at. Would showInITunesStore be better?

showInITunesStore looks better.

And if so should runJavascriptOnWebPage be runJavaScriptOnWebPage?

The original shown name is "Run JavaScript on Web Page" if I recall correctly, so runJavaScriptOnWebPage should be the right one.

@joshfarrant
Copy link
Owner Author

Cool, I’ll adjust those too then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion Discussion of future features
Projects
None yet
Development

No branches or pull requests

2 participants