Summary

[Emoji UI Picker] [PART 2 | Djblets] Finished working implementation

Review Request #11880 — Created Nov. 21, 2021 and updated Dec. 6, 2021, 10:01 p.m.

Information

Owner

matthewgoodman13

Repository

Djblets

Branch

release-2.x

Bugs

Depends On

Commit

a864bc5...

Reviewers

Groups

djblets

People

Description

[Emoji UI Picker] [PART 2 | Djblets] Finished working implementation
See Final Report

Testing Done

=> Local testing

=> Unit tests added for emoji registry methods

Issues

Description	From	Last Updated
E501 line too long (86 > 79 characters)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E261 at least two spaces before inline comment	reviewbot	Nov. 21, 2021, 9:43 p.m.
E501 line too long (91 > 79 characters)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E501 line too long (82 > 79 characters)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W291 trailing whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W291 trailing whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E117 over-indented	reviewbot	Nov. 21, 2021, 9:43 p.m.
E501 line too long (102 > 79 characters)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E501 line too long (91 > 79 characters)	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:43 p.m.
E117 over-indented	reviewbot	Nov. 21, 2021, 9:43 p.m.
W291 trailing whitespace	reviewbot	Nov. 21, 2021, 9:43 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (92 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
W291 trailing whitespace	reviewbot	Nov. 21, 2021, 9:48 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E201 whitespace after '{'	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (97 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E202 whitespace before '}'	reviewbot	Nov. 21, 2021, 9:48 p.m.
W292 no newline at end of file	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (32480 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
W292 no newline at end of file	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (107 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 21, 2021, 9:48 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 21, 2021, 9:48 p.m.
This isn't something we need to worry about. Djblets 2.x+ is Django 1.11+ only.	david	Nov. 24, 2021, 8:50 p.m.
A few things here with imports: The first import should always be unicode_literals as well as any other __future__ things …	david	Nov. 24, 2021, 8:57 p.m.
Parens aren't necessary here	david	Nov. 24, 2021, 8:58 p.m.
No need to define the local variable here. Just assign directly into the dict key.	david	Nov. 24, 2021, 8:58 p.m.
This isn't a helpful thing to have here--it's kind of obvious what these are.	david	Nov. 24, 2021, 8:58 p.m.
Combine these lines ("""Add the src attributes...)	david	Nov. 24, 2021, 9:02 p.m.
For docstrings, the first line should be a summary, and additional info should go in other paragraphs. At the very …	david	Nov. 24, 2021, 9:04 p.m.
All identifiers in python (except for class names) should use underscores rather than camelCase.	david	Nov. 24, 2021, 9:31 p.m.
This isn't particularly efficient (.index() has to iterate the list every time it's called). When we're building the sort order, …	david	Nov. 24, 2021, 9:28 p.m.
There's a python syntax trick that lets us avoid defining a method for this: combined_indices = {i1['emoji'], i2['emoji']} combined_aliases = …	david	Nov. 24, 2021, 9:35 p.m.
Python style says that this should be named get_unicode_sort_order	david	Nov. 24, 2021, 9:35 p.m.
We really should make this process the .txt file directly from the unicode consortium rather than list it out ourselves. …	david	Nov. 24, 2021, 9:35 p.m.
E501 line too long (32480 > 79 characters)	reviewbot	Nov. 24, 2021, 9:45 p.m.
Same comments about import orders.	david	Nov. 24, 2021, 9:35 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 24, 2021, 9:45 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 24, 2021, 9:45 p.m.
All string literals should use single quotes unless there's a good reason not to (for example, a single quote within …	david	Nov. 24, 2021, 9:37 p.m.
Comments should use #, not triple-quoted strings.	david	Nov. 24, 2021, 9:38 p.m.
E501 line too long (107 > 79 characters)	reviewbot	Nov. 24, 2021, 9:45 p.m.
Based on my reading of the code, I think this can just be: index = emoji_registry.get_emoji_index(for_extension=True) new_kwargs['emoji_index'] = lambda: index	david	Nov. 24, 2021, 9:38 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 24, 2021, 9:45 p.m.
Make sure to add info.	david	Nov. 24, 2021, 9:38 p.m.
This needs a "Returns" section.	david	Nov. 24, 2021, 9:39 p.m.
This needs a "Returns" section.	david	Nov. 24, 2021, 9:39 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
W291 trailing whitespace	reviewbot	Nov. 28, 2021, 1:01 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 28, 2021, 1:01 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Nov. 28, 2021, 1:01 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 28, 2021, 1:01 p.m.
W292 no newline at end of file	reviewbot	Nov. 28, 2021, 1:01 p.m.
F401 'kgb.SpyAgency' imported but unused	reviewbot	Nov. 28, 2021, 1:01 p.m.
F401 'djblets.siteconfig.models.SiteConfiguration' imported but unused	reviewbot	Nov. 28, 2021, 1:01 p.m.
F401 'djblets.testing.testcases.TestCase' imported but unused	reviewbot	Nov. 28, 2021, 1:01 p.m.
E303 too many blank lines (3)	reviewbot	Nov. 28, 2021, 1:01 p.m.
W391 blank line at end of file	reviewbot	Nov. 28, 2021, 1:01 p.m.
F401 'collections.OrderedDict' imported but unused	reviewbot	Nov. 28, 2021, 1:01 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 28, 2021, 1:01 p.m.
W293 blank line contains whitespace	reviewbot	Nov. 28, 2021, 1:01 p.m.
W292 no newline at end of file	reviewbot	Nov. 28, 2021, 1:01 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 28, 2021, 1:01 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
W291 trailing whitespace	reviewbot	Nov. 28, 2021, 1:06 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Nov. 28, 2021, 1:04 p.m.
E303 too many blank lines (3)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 28, 2021, 1:06 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 28, 2021, 1:06 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (97 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E303 too many blank lines (3)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Dec. 6, 2021, 9:54 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Dec. 6, 2021, 9:55 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Dec. 6, 2021, 9:55 p.m.
F401 'pytz' imported but unused	reviewbot	Dec. 6, 2021, 9:55 p.m.
E501 line too long (97 > 79 characters)	reviewbot	Dec. 6, 2021, 9:55 p.m.
E303 too many blank lines (2)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E501 line too long (86 > 79 characters)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E501 line too long (90 > 79 characters)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E501 line too long (92 > 79 characters)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E303 too many blank lines (2)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E501 line too long (101 > 79 characters)	reviewbot	Dec. 6, 2021, 9:58 p.m.
E501 line too long (101 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E303 too many blank lines (2)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E303 too many blank lines (2)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Dec. 6, 2021, 9:59 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Dec. 6, 2021, 10:01 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.
E501 line too long (95 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.
E501 line too long (98 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Dec. 6, 2021, 10:01 p.m.

flake8 failed.

JSHint passed.

flake8

```
Warning: Showing 30 of 49 failures.
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E501 line too long (86 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E261 at least two spaces before inline comment
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E501 line too long (91 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E501 line too long (82 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E117 over-indented
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E501 line too long (102 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E501 line too long (91 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
E117 over-indented
```
djblets/emojis/registry.py (Diff revision 1)
Show all issues
```
W291 trailing whitespace
```

Commit:

-	cbcba6581b831d2e904fd3c7e722074cbff25b52
+	dfb23c9363580d00e0de7a5abf35c14ce26a23cc

Diff:

Revision 2 (+446)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/apps.py
	djblets/emojis/registry.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E501 line too long (92 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E501 line too long (80 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E201 whitespace after '{'
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E501 line too long (97 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
E202 whitespace before '}'
```
djblets/emojis/registry.py (Diff revision 2)
Show all issues
```
W292 no newline at end of file
```
djblets/emojis/unicode_sort_order.py (Diff revision 2)
Show all issues
```
E501 line too long (32480 > 79 characters)
```
djblets/emojis/unicode_sort_order.py (Diff revision 2)
Show all issues
```
W292 no newline at end of file
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
Show all issues
```
E501 line too long (107 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
Show all issues
```
E501 line too long (80 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```

Commit:

-	dfb23c9363580d00e0de7a5abf35c14ce26a23cc
+	addf68d15814eef3b3c01c4b7fc3d6ca463171ec

Diff:

Revision 3 (+449)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/apps.py
	djblets/emojis/registry.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/unicode_sort_order.py (Diff revision 3)
Show all issues
```
E501 line too long (32480 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
E501 line too long (107 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Description:

		[WIP \| Emoji UI Picker] [PART 2 \| Djblets] Working implementation ready for review

		=> Would like a preliminary review
~		=> Still need to add Unit tests
	~	=> Still need to add unit tests

OK, I added a handful of comments (some of these are style things that apply throughout the change and I just marked one instance).

The biggest thing is how we're dealing with the sort order. We really should just take the .txt file from unicode.org and bundle it, and then have our code process that file to build the sort order. That way, when future versions of Unicode come out, we can just drop in a new file.

djblets/emojis/apps.py (Diff revision 3)

Show all issues

This isn't something we need to worry about. Djblets 2.x+ is Django 1.11+ only.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

A few things here with imports:
The first import should always be unicode_literals as well as any other __future__ things that are necessary. After that there are three groups: standard library, third-party, and the same module. Within each group we alphabetize. We also don't use the relative import syntax (yet).

from __future__ import unicode_literals

import datetime
import json
from collections import OrderedDict

import pytz

from djblets.emojis.unicode_sort_order import get_unicode_sort_order
from djblets.siteconfig.models import SiteConfiguration

djblets/emojis/registry.py (Diff revision 3)
Show all issues
```
Parens aren't necessary here
```

djblets/emojis/registry.py (Diff revision 3)

Show all issues

No need to define the local variable here. Just assign directly into the dict key.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

This isn't a helpful thing to have here--it's kind of obvious what these are.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

Combine these lines ("""Add the src attributes...)

djblets/emojis/registry.py (Diff revision 3)

Show all issues

For docstrings, the first line should be a summary, and additional info should go in other paragraphs. At the very least put a blank line between these two and combine the """ with the summary line.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

All identifiers in python (except for class names) should use underscores rather than camelCase.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

This isn't particularly efficient (.index() has to iterate the list every time it's called). When we're building the sort order, let's create a dict mapping the codepoint to its index value.

djblets/emojis/registry.py (Diff revision 3)

Show all issues

There's a python syntax trick that lets us avoid defining a method for this:

combined_indices = {**i1['emoji'], **i2['emoji']}
combined_aliases = {**i1['aliases'], **i2['aliases']}

djblets/emojis/unicode_sort_order.py (Diff revision 3)

Show all issues

Python style says that this should be named get_unicode_sort_order

djblets/emojis/unicode_sort_order.py (Diff revision 3)

Show all issues

We really should make this process the .txt file directly from the unicode consortium rather than list it out ourselves. That way it's much easier to update.

djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)

Show all issues

Same comments about import orders.

djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)

Show all issues

All string literals should use single quotes unless there's a good reason not to (for example, a single quote within the string content itself).

djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
Comments should use #, not triple-quoted strings.
```

djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)

Show all issues

Based on my reading of the code, I think this can just be:

index = emoji_registry.get_emoji_index(for_extension=True)
new_kwargs['emoji_index'] = lambda: index

djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
Make sure to add info.
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
This needs a "Returns" section.
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
Show all issues
```
This needs a "Returns" section.
```

Change Summary:

David's comments + changes to how emoji index is retrieved for optimization

Commit:

-	addf68d15814eef3b3c01c4b7fc3d6ca463171ec
+	757311866922ce1b6962f4cf40a35a985bf59127

Diff:

Revision 4 (+4217)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 4)
Show all issues
```
W292 no newline at end of file
```
djblets/emojis/tests.py (Diff revision 4)
Show all issues
```
F401 'kgb.SpyAgency' imported but unused
```

djblets/emojis/tests.py (Diff revision 4)

Show all issues

F401 'djblets.siteconfig.models.SiteConfiguration' imported but unused

djblets/emojis/tests.py (Diff revision 4)

Show all issues

F401 'djblets.testing.testcases.TestCase' imported but unused

djblets/emojis/tests.py (Diff revision 4)
Show all issues
```
E303 too many blank lines (3)
```
djblets/emojis/tests.py (Diff revision 4)
Show all issues
```
W391 blank line at end of file
```
djblets/emojis/unicode_sort_order.py (Diff revision 4)
Show all issues
```
F401 'collections.OrderedDict' imported but unused
```
djblets/emojis/unicode_sort_order.py (Diff revision 4)
Show all issues
```
E302 expected 2 blank lines, found 1
```
djblets/emojis/unicode_sort_order.py (Diff revision 4)
Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/unicode_sort_order.py (Diff revision 4)
Show all issues
```
W292 no newline at end of file
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E302 expected 2 blank lines, found 1
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 4)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Commit:

-	757311866922ce1b6962f4cf40a35a985bf59127
+	83d8ae7178776605beeff367e6e5bf75588bd371

Diff:

Revision 5 (+4218)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 5)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 5)
Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 5)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/emojis/tests.py (Diff revision 5)
Show all issues
```
E303 too many blank lines (3)
```
djblets/emojis/unicode_sort_order.py (Diff revision 5)
Show all issues
```
E302 expected 2 blank lines, found 1
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E302 expected 2 blank lines, found 1
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 5)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Commit:

-	83d8ae7178776605beeff367e6e5bf75588bd371
+	5e1a537147aac5e39151b0091d441144525f5f01

Diff:

Revision 6 (+4218)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 6)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 6)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 6)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 6)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 6)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 6)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 6)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Description:

		[WIP \| Emoji UI Picker] [PART 2 \| Djblets] Working implementation ready for review

		=> Would like a preliminary review
		=> Still need to add unit tests
	+
	+

Commit:

-	5e1a537147aac5e39151b0091d441144525f5f01
+	33634418a4b43d4aa3f0acda5ddd1a5c3aee395d

Diff:

Revision 7 (+4295)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py
	docs/releasenotes/2.0.1.rst

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 7)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 7)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/emojis/tests.py (Diff revision 7)
Show all issues
```
E501 line too long (97 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 7)
Show all issues
```
E303 too many blank lines (3)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 7)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 7)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 7)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 7)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 7)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Commit:

-	33634418a4b43d4aa3f0acda5ddd1a5c3aee395d
+	a5f50f622be37879c7a32741a7889bb57db02cdd

Diff:

Revision 8 (+4389)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 8)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 8)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
F401 'pytz' imported but unused
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (97 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (86 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (90 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (92 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (101 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E501 line too long (101 > 79 characters)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/tests.py (Diff revision 8)
Show all issues
```
E303 too many blank lines (2)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 8)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 8)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 8)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 8)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 8)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Summary:

-	[WIP \| Emoji UI Picker] [PART 2 \| Djblets] Working implementation ready for review
+	[Emoji UI Picker] [PART 2 \| Djblets] Finished working implementation

Description:

~		[WIP \| Emoji UI Picker] [PART 2 \| Djblets] Working implementation ready for review
	~	[Emoji UI Picker] [PART 2 \| Djblets] Finished working implementation

~		=> Would like a preliminary review
	~	See Final Report
-		=> Still need to add unit tests

Testing Done:

		=> Local testing
~		=> Unit tests needed
	~	=> Unit tests added for emoji registry methods

Commit:

-	a5f50f622be37879c7a32741a7889bb57db02cdd
+	a864bc5f85b755534984dffe5f122710f64d4432

Diff:

Revision 9 (+4386)

Show changes

	djblets/emojis/__init__.py
	djblets/emojis/registry.py
	djblets/emojis/sort_order.txt
	djblets/emojis/tests.py
	djblets/emojis/unicode_sort_order.py
	djblets/markdown/extensions/emoji_extension_wrapper.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/emojis/registry.py (Diff revision 9)
Show all issues
```
E501 line too long (84 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 9)
Show all issues
```
E999 SyntaxError: invalid syntax
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 9)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 9)
Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 9)
Show all issues
```
E501 line too long (95 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 9)
Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 9)
Show all issues
```
E501 line too long (80 > 79 characters)
```