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)
The issue has been dropped. Show all issues
```
E501 line too long (86 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E261 at least two spaces before inline comment
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (91 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (82 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E117 over-indented
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (102 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (91 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E117 over-indented
```
djblets/emojis/registry.py (Diff revision 1)
The issue has been dropped. 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)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (92 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W291 trailing whitespace
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E303 too many blank lines (2)
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (80 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E201 whitespace after '{'
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (97 > 79 characters)
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E202 whitespace before '}'
```
djblets/emojis/registry.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W292 no newline at end of file
```
djblets/emojis/unicode_sort_order.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (32480 > 79 characters)
```
djblets/emojis/unicode_sort_order.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W292 no newline at end of file
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (107 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (80 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 2)
The issue has been dropped. 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)
The issue has been dropped. Show all issues
```
E501 line too long (32480 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E501 line too long (98 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E501 line too long (81 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E501 line too long (107 > 79 characters)
```
djblets/markdown/extensions/emoji_extension_wrapper.py (Diff revision 3)
The issue has been dropped. 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)