Skip to content
This repository has been archived by the owner on Nov 16, 2020. It is now read-only.

Update DateFormat.swift #37

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update DateFormat.swift #37

wants to merge 4 commits into from

Conversation

MihaelIsaev
Copy link
Member

Add ability to set locale as third parameter

MihaelIsaev and others added 3 commits August 19, 2018 14:57
Add ability to set locale as third parameter
Change error text
Also add ability to set time zone
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

LGTM, just some small changes.

case 1, 2: break
default: throw tag.error(reason: "Invalid parameter count: \(tag.parameters.count). 1 or 2 required.")
case 1, 2, 3, 4: break
default: throw tag.error(reason: "Invalid parameter count: \(tag.parameters.count). 1 or 2 or 3 or 4 required.")
Copy link
Member

Choose a reason for hiding this comment

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

Would be a bit less awkward to read as:

1, 2, 3, or 4 required

@@ -11,8 +11,8 @@ public final class DateFormat: TagRenderer {
public func render(tag: TagContext) throws -> Future<TemplateData> {
/// Require at least one parameter.
switch tag.parameters.count {
case 1, 2: break
default: throw tag.error(reason: "Invalid parameter count: \(tag.parameters.count). 1 or 2 required.")
case 1, 2, 3, 4: break
Copy link
Member

Choose a reason for hiding this comment

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

This could be better expressed as 1...4

@tanner0101 tanner0101 added the enhancement New feature or request label Sep 17, 2018
@tanner0101 tanner0101 self-assigned this Sep 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
No open projects
Vapor 3
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants