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

Setting a default format #48

Closed
wants to merge 2 commits into from
Closed

Setting a default format #48

wants to merge 2 commits into from

Conversation

jacobthetechy
Copy link

I found in using this in my app I use the same string format in all my web pages. When adding new dates in new pages I would have a hard time remembering what exactly that format was. This solves that problem by setting a config variable to the format needed without needing to register a new jinja variable.

jacobthetechy added 2 commits June 13, 2019 12:14
@miguelgrinberg
Copy link
Owner

Thanks for the contribution. The main issue that I have is that the design of this extension is such that in general the methods of this extension match the JavaScript functions in moment.js. Adding a default() method alongside format(), fromNow(), etc. makes it look like this is also part of moment.js.

@jacobthetechy
Copy link
Author

I fully understand that.

What about this. In moment.js if you call moment().format() it will yield the current time in ISO 8601 format. If you do the same with flask-moment you get
TypeError: format() missing 1 required positional argument: 'fmt'
To be closer to how moment.js works would this be a accepted change?

default_moment_version = '2.24.0'
default_moment_sri = 'sha256-AdQN98MVZs44Eq2yTwtoKufhnU+uZ7v2kXnD5vqzZVo='
default_format = 'YYYY-MM-DDTHH:mm:ssZ'  # ISO 8601 from http://bit.ly/2FdZ5HA

##################################################################

def format(self, fmt=None, refresh=False):
        if fmt is None:
            fmt = current_app.config.get('MOMENT_DEFAULT_FORMAT', default_fmt)

        return self._render("format('%s')" % fmt, refresh)

@miguelgrinberg
Copy link
Owner

@jacobthetechy yes, supporting format() w/o arguments is reasonable, I actually wasn't aware of that option. But can we let moment.js handle the default format instead of duplicating that logic on the Python side?

@jacobthetechy
Copy link
Author

@miguelgrinberg This is what I have come up with.

@staticmethod
def defaultFormat(fmt):
    return Markup('<script>\nmoment.defaultFormat = "%s";\n</script>' % fmt)

##################################################################

def format(self, fmt='', refresh=False):
    return self._render("format('%s')" % fmt, refresh)

To make the change you add {{moment.defaultFormat('LT')}} under {{ moment.include_moment() }}
Only difference here instead of calling moment.defaultFormat = "format_string"; per moment.js you use the function moment.defaultFormat('format_str') for the python code.

I believe its the only way for python, javascript, and jinja to all be happy. I tried to be as close to moment.js but jinja does not like {{moment.defaultFormat = "format_string"}} with using python getters and setters functions.

Thoughts?

@miguelgrinberg
Copy link
Owner

@jacobthetechy Thanks! I took parts of your PR and completed this feature. Take a look at the master branch.

@jacobthetechy
Copy link
Author

@miguelgrinberg Beautiful! Thanks so much for adding this and allowing me to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants