Add a setting for mapping file extensions to syntax highlighters for diffs

Review Request #11821 — Created Sept. 24, 2021 and submitted

Information

Review Board
release-5.0.x

Reviewers

Previously, we guess what syntax highlighting to apply to files in the diff
viewer given their data and file extension. Sometimes this guess would be wrong,
and we have no way of setting what type of syntax highlighting gets applied to
files. This change adds a setting for mapping file extensions to pygments
lexers (the syntax highlighters), namely
reviewboard.admin.siteconfig.diffviewer_custom_pygments_lexers. If a mapping
is not defined for a certain file extension, we revert to the old behaviour
of guessing which lexer to use given the filename and file contents.

Adds reviewboard.admin.form_widgets.LexersMappingWidget which partly
handles the UI for modifying this mapping.

  • Created unit tests for the mapping in
    reviewboard.diffviewer.tests.test_raw_diff_chunk_generator.
    Ran all unit tests in reviewboard.diffviewer with success
  • Created the unit tests for the LexersMappingWidget in
    reviewboard.admin.tests.test_lexers_mapping_widget.
    Ran all unit tests in reviewboard.admin.tests with success.
Summary ID Author
resolved merge conflicts from reviewboard-5.0.x
updates
aac35dd214d59c3a66996bae103eaee99691960d Michelle
Description From Last Updated

Since we're using both the key and the value, the items() iterator might be nicer than keys(): for ext, lexer_name …

daviddavid

In the case where there's a configured lexer that matches an extension, but that doesn't exist in Pygments, we probably …

daviddavid

Python actually has a really nice syntax feature where for has an else clause that gets executed in the case …

daviddavid

W291 trailing whitespace

reviewbotreviewbot

E122 continuation line missing indentation or outdented

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

E122 continuation line missing indentation or outdented

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

E122 continuation line missing indentation or outdented

reviewbotreviewbot

This doesn't seem valuable. Let's just use the one that we imported.

daviddavid

Please add a blank line between these.

daviddavid

Add a blank line here too.

daviddavid

The logging messages do their own format operation on the arguments, so we don't have to use % here. Let's …

daviddavid

Ahh, I see now why you were defining the class local version. Django has a helper for this that you …

daviddavid

W293 blank line contains whitespace

reviewbotreviewbot

E123 closing bracket does not match indentation of opening bracket's line

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W292 no newline at end of file

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W292 no newline at end of file

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

Let's try to shorten this to be a single line. Additional detail can be added in the next paragraph if …

daviddavid

Leading underscore isn't needed here.

daviddavid

Dedent 4 spaces.

daviddavid

Dedent 4 spaces.

daviddavid

We should probably clarify in the help text that this is for pygments so people can find the appropriate documentation.

daviddavid

Let's use parens and string concatenation instead of the triple-quoted string here: correct_html = ( '<input type...' '<select name...' )

daviddavid

Can wrap this a little bit nicer: correct_html += format_html( '...') That said, string concatenation in Python isn't particularly efficient. …

daviddavid

Same comments here as in above method.

daviddavid

Keep imports in alphabetical order.

daviddavid

Let's swap these so they're alphabetical

daviddavid

Add a blank line here.

daviddavid

Please alphabetize.

daviddavid

Add a blank line before these. Let's also put c before j

daviddavid

Alphabetize.

daviddavid

Add a blank line in here.

daviddavid

Add a blank line between these two.

daviddavid

This comment just says exactly what the code says, so it's not adding much information. Let's get rid of it.

daviddavid

This should go in its own import section above the reviewboard imports.

daviddavid

We have a context manager that might dramatically simplify this test class. It will do a temporary override of the …

daviddavid

F811 redefinition of unused 'render_to_string' from line 6

reviewbotreviewbot

Add a blank line between these two.

daviddavid

This needs to just be a single line. If we need more detail, we can add another paragraph.

daviddavid

should I add a "Version added: 5.0.0" in here?

maubinmaubin

Since the entire class is added in 5.0, I don't think we need version specifiers for each method inside it. …

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
  1. Looks like a great start!

  2. reviewboard/diffviewer/chunk_generator.py (Diff revision 1)
     
     
     
     
    Show all issues

    Since we're using both the key and the value, the items() iterator might be nicer than keys():

    for ext, lexer_name in CUSTOM_PYGMENTS_LEXERS.items():
        ...
    
  3. Show all issues

    In the case where there's a configured lexer that matches an extension, but that doesn't exist in Pygments, we probably should log an error so the user knows what went wrong.

  4. reviewboard/diffviewer/chunk_generator.py (Diff revision 1)
     
     
     
     
    Show all issues

    Python actually has a really nice syntax feature where for has an else clause that gets executed in the case where nothing called break. So this could be combined:

    lexer = None
    
    for ext = CUSTOM_PYGMENTS_LEXERS.keys():
        # Try to instantiate lexer
        ...
    else:
        # Use guess_lexer_for_filename
    
  5. 
      
maubin
Review request changed
Summary:
[WIP] Add a setting for mapping file extensions to syntax highlighters for diffs
Add a setting for mapping file extensions to syntax highlighters for diffs
Testing Done:
~  

None yet.

  ~

Created the following unit tests in reviewboard.diffviewer.tests.test_raw_diff_chunk_generator:

  + - test_apply_pygments_with_custom_mapping() which tests if custom lexer mapping works and if it overrides the old behaviour of guessing
  + - test_apply_pygments_with_bad_custom_mapping() which tests if proper error is logged when the custom mapping maps an extension to a non existant lexer class

  +
  +

Ran all unit tests in reviewboard.diffviewer with success

Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
david
  1. 
      
  2. reviewboard/diffviewer/chunk_generator.py (Diff revision 4)
     
     
     
    Show all issues

    This doesn't seem valuable. Let's just use the one that we imported.

  3. reviewboard/diffviewer/chunk_generator.py (Diff revision 4)
     
     
     
    Show all issues

    Please add a blank line between these.

  4. reviewboard/diffviewer/chunk_generator.py (Diff revision 4)
     
     
     
    Show all issues

    Add a blank line here too.

  5. reviewboard/diffviewer/chunk_generator.py (Diff revision 4)
     
     
     
     
     
     
    Show all issues

    The logging messages do their own format operation on the arguments, so we don't have to use % here. Let's also make this message a little bit more explicit/terse:

    logger.error('Pygments lexer "%s" for "%s" files in '
                 'settings.CUSTOM_PYGMENTS_LEXERS was not found.',
                 lexer_name, ext)
    
  6. Show all issues

    Ahh, I see now why you were defining the class local version. Django has a helper for this that you can use as a decorator: django.test.utils.override_settings:

    @override_settings(CUSTOM_PYGMENTS_LEXERS={'.md': 'LessCss'})
    def test_apply_pygments_with_custom_mapping(self):
        ...
    
  7. 
      
maubin
Review request changed
Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
maubin
Review request changed
Summary:
Add a setting for mapping file extensions to syntax highlighters for diffs
[WIP] Add a setting for mapping file extensions to syntax highlighters for diffs
Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle
fixed a couple flake8 issues
d1a3b05fd934f96b700bb87b60e2613e89df833f Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle
fixed a couple flake8 issues
d1a3b05fd934f96b700bb87b60e2613e89df833f Michelle
moved mapping to siteconfig instead of settings.py
02756419d83742c1b0181d806bdb57b3cd6661aa Michelle
initial form for mapping in diff viewer settings
fbd7700720326e780e696c44181650451beaad95 Michelle
added test for empty mapping
faa04403111580a7b8494fc726904764887bac9a Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Change Summary:

Create LexersMappingWidget and using it in diff settings to test for now

Testing Done:
   

Created the following unit tests in reviewboard.diffviewer.tests.test_raw_diff_chunk_generator:

    - test_apply_pygments_with_custom_mapping() which tests if custom lexer mapping works and if it overrides the old behaviour of guessing
~   - test_apply_pygments_with_bad_custom_mapping() which tests if proper error is logged when the custom mapping maps an extension to a non existant lexer class

  ~ - test_apply_pygments_with_bad_custom_mapping() which tests if proper error is logged when the custom mapping maps an extension to a non existant lexer class
  + - test_apply_pygments_with_empty_custom_mapping() which tests if apply_pygments still works as expected with an empty custom mapping.

   
   

Ran all unit tests in reviewboard.diffviewer with success

Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle
fixed a couple flake8 issues
d1a3b05fd934f96b700bb87b60e2613e89df833f Michelle
moved mapping to siteconfig instead of settings.py
02756419d83742c1b0181d806bdb57b3cd6661aa Michelle
initial form for mapping in diff viewer settings
fbd7700720326e780e696c44181650451beaad95 Michelle
added test for empty mapping
faa04403111580a7b8494fc726904764887bac9a Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle
fixed a couple flake8 issues
d1a3b05fd934f96b700bb87b60e2613e89df833f Michelle
moved mapping to siteconfig instead of settings.py
02756419d83742c1b0181d806bdb57b3cd6661aa Michelle
initial form for mapping in diff viewer settings
fbd7700720326e780e696c44181650451beaad95 Michelle
added test for empty mapping
faa04403111580a7b8494fc726904764887bac9a Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
48047abe632054dd35db53369725cb063e7f3ff4 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Change Summary:

took out changes from bad rebasing

Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
3e4ddeda899f0e5d97f02ed070d7525662ee33c5 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
97a6bd3547ff2c3af0708640538d75ef4a55f1c8 Michelle
Logs an error if a mapped lexer does not exist in Pygments
13f279fa09f30803a02621e13192942b95e80460 Michelle
Removed trailing whitespace
d27cc3d581a53a7fd414db5efc820cba92ba9fb3 Michelle
Properly imports mapping from settings.py
8eb445508de1a38aae178bc59477b585f681bbd6 Michelle
fixed string formatting for logged error
dafacdae4c204636121480ab9b3821ffec9ff889 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
45754bd19bf4ae4ae6f9582e725557ddf0e3c976 Michelle
took out print statement from testing
5fb349b3f4d0adb8a4f0c0b25d3df3611a446220 Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
c29591ba2ab98206ea65f12b248826d2e5ad5eac Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
f853203d6bcadd51d4e9dcfe9561607034023ab7 Michelle
fixed flake8 issues
f734b34e8ddc5eca2a3000f71feab356c0541cfd Michelle
fixed one last flake8 indent issue
d3dd8e59f73f2c246e6f04ab57c4b2dbfaa87f4b Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
5e248bac7d732347961213f7a83934a07c604426 Michelle
fixed a couple flake8 issues
d1a3b05fd934f96b700bb87b60e2613e89df833f Michelle
moved mapping to siteconfig instead of settings.py
02756419d83742c1b0181d806bdb57b3cd6661aa Michelle
initial form for mapping in diff viewer settings
fbd7700720326e780e696c44181650451beaad95 Michelle
added test for empty mapping
faa04403111580a7b8494fc726904764887bac9a Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
48047abe632054dd35db53369725cb063e7f3ff4 Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Change Summary:

Add tests for LexersMappingWidget

Description:
   

Adds a default mapping of file extensions to pygments lexers, which

    administrators may modify to define their own custom mapping. If a
    mapping is not defined for a certain file extension, we revert to
    the old behaviour of guessing which lexer to use given the filename
    and file contents.

  +
  +

Adds reviewboard.admin.form_widgets.LexersMappingWidget which partly handles

  + the UI for maintaining this mapping.

Testing Done:
   

Created the following unit tests in reviewboard.diffviewer.tests.test_raw_diff_chunk_generator:

    - test_apply_pygments_with_custom_mapping() which tests if custom lexer mapping works and if it overrides the old behaviour of guessing
    - test_apply_pygments_with_bad_custom_mapping() which tests if proper error is logged when the custom mapping maps an extension to a non existant lexer class
    - test_apply_pygments_with_empty_custom_mapping() which tests if apply_pygments still works as expected with an empty custom mapping.

   
   

Ran all unit tests in reviewboard.diffviewer with success

  +
  +

Created the unit tests for the LexersMappingWidget in reviewboard.admin.tests.test_lexers_mapping_widget.

  + Ran all unit tests in reviewboard.admin.tests with success.

Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle
testing listeditwidget with custom pygments lexers setting
38a50c83e212b487d4d62537b0fa64c9c6cbe2ff Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
Change Summary:

Everything is done and tested, except for handling local site stuff

Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle
testing listeditwidget with custom pygments lexers setting
38a50c83e212b487d4d62537b0fa64c9c6cbe2ff Michelle
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle
testing listeditwidget with custom pygments lexers setting
38a50c83e212b487d4d62537b0fa64c9c6cbe2ff Michelle
add documentation and finalize code
8d4a6d7996c73bee56335d8f5ea55f16d4aee271 Michelle

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
david
  1. 
      
  2. reviewboard/admin/form_widgets.py (Diff revision 13)
     
     
     
    Show all issues

    Let's try to shorten this to be a single line. Additional detail can be added in the next paragraph if necessary.

  3. reviewboard/admin/form_widgets.py (Diff revision 13)
     
     
    Show all issues

    Leading underscore isn't needed here.

  4. reviewboard/admin/form_widgets.py (Diff revision 13)
     
     
    Show all issues

    Dedent 4 spaces.

  5. reviewboard/admin/form_widgets.py (Diff revision 13)
     
     
    Show all issues

    Dedent 4 spaces.

  6. reviewboard/admin/forms/diff_settings.py (Diff revision 13)
     
     
     
    Show all issues

    We should probably clarify in the help text that this is for pygments so people can find the appropriate documentation.

  7. Show all issues

    Let's use parens and string concatenation instead of the triple-quoted string here:

    correct_html = (
        '<input type...'
        '<select name...'
    )
    
  8. Show all issues

    Can wrap this a little bit nicer:

    correct_html += format_html(
        '...')
    

    That said, string concatenation in Python isn't particularly efficient. The usual convention is to build a list and join all at once:

    correct_html_parts = [
        '<input ...',
        '<select...',
    ]
    
    for lex in get_all_lexers():
        ...
        correct_html_parts.append(format_html(
            '...'))
    
    correct_html_parts.append('</select>')
    correct_html = ''.join(correct_html_parts)
    
  9. Show all issues

    Same comments here as in above method.

  10. reviewboard/diffviewer/chunk_generator.py (Diff revision 13)
     
     
     
     
    Show all issues

    Keep imports in alphabetical order.

  11. reviewboard/diffviewer/chunk_generator.py (Diff revision 13)
     
     
     
    Show all issues

    Let's swap these so they're alphabetical

  12. 
      
maubin
david
  1. 
      
  2. reviewboard/admin/form_widgets.py (Diff revision 14)
     
     
     
    Show all issues

    Add a blank line here.

  3. reviewboard/admin/forms/diff_settings.py (Diff revision 14)
     
     
     
     
    Show all issues

    Please alphabetize.

  4. reviewboard/admin/forms/diff_settings.py (Diff revision 14)
     
     
     
    Show all issues

    Add a blank line before these. Let's also put c before j

  5. Show all issues

    Alphabetize.

  6. Show all issues

    Add a blank line in here.

  7. Show all issues

    Add a blank line between these two.

  8. Show all issues

    This comment just says exactly what the code says, so it's not adding much information. Let's get rid of it.

  9. Show all issues

    This should go in its own import section above the reviewboard imports.

  10. reviewboard/diffviewer/tests/test_raw_diff_chunk_generator.py (Diff revision 14)
     
     
     
     
     
     
     
     
    Show all issues

    We have a context manager that might dramatically simplify this test class. It will do a temporary override of the siteconfig and then restore the old settings when that scope is exited. Can you try this and see if it does the right thing?

    ```python
    settings = {
    'diffviewer_custom_pygments_lexers': {'.md', 'LessCss'},
    }

    with self.siteconfig_settings(settings):
    chunk_generator = ....

    1. Tried it out and ran into this error.

      ERROR: Testing RawDiffChunkGenerator._apply_pygments with a custom lexer mapping
      ----------------------------------------------------------------------
      Traceback (most recent call last):
        File "/home/maubin/reviewboard/src/rb-master/reviewboard/reviewboard/diffviewer/tests/test_raw_diff_chunk_generator.py", line 478, in test_apply_pygments_with_custom_mapping
          with self.siteconfig_settings(settings):
        File "/home/maubin/.pyenv/versions/3.9.7/lib/python3.9/contextlib.py", line 119, in __enter__
          return next(self.gen)
        File "/home/maubin/reviewboard/src/rb-master/reviewboard/reviewboard/testing/testcase.py", line 1904, in siteconfig_settings
          with super(TestCase, self).siteconfig_settings(settings):
        File "/home/maubin/.pyenv/versions/3.9.7/lib/python3.9/contextlib.py", line 119, in __enter__
          return next(self.gen)
        File "/home/maubin/reviewboard/src/rb-master/djblets/djblets/testing/testcases.py", line 162, in siteconfig_settings
          siteconfig.save()
        File "/home/maubin/reviewboard/src/rb-master/djblets/djblets/siteconfig/models.py", line 309, in save
          super(SiteConfiguration, self).save(**kwargs)
        File "/home/maubin/envs/reviewboard-dev/lib/python3.9/site-packages/django/db/models/base.py", line 807, in save
          self.save_base(using=using, force_insert=force_insert,
        File "/home/maubin/envs/reviewboard-dev/lib/python3.9/site-packages/django/db/models/base.py", line 838, in save_base
          updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
        File "/home/maubin/envs/reviewboard-dev/lib/python3.9/site-packages/django/db/models/base.py", line 901, in _save_table
          values = [(f, None, (getattr(self, f.attname) if raw else f.pre_save(self, False)))
        File "/home/maubin/envs/reviewboard-dev/lib/python3.9/site-packages/django/db/models/base.py", line 901, in <listcomp>
          values = [(f, None, (getattr(self, f.attname) if raw else f.pre_save(self, False)))
        File "/home/maubin/reviewboard/src/rb-master/djblets/djblets/db/fields/json_field.py", line 244, in pre_save
          return self.dumps(getattr(model_instance, self.attname, None))
        File "/home/maubin/reviewboard/src/rb-master/djblets/djblets/db/fields/json_field.py", line 352, in dumps
          return self.encoder.encode(data)
        File "/home/maubin/.pyenv/versions/3.9.7/lib/python3.9/json/encoder.py", line 199, in encode
          chunks = self.iterencode(o, _one_shot=True)
        File "/home/maubin/.pyenv/versions/3.9.7/lib/python3.9/json/encoder.py", line 257, in iterencode
          return _iterencode(o, 0)
        File "/home/maubin/reviewboard/src/rb-master/djblets/djblets/util/serializers.py", line 77, in default
          return super(DjbletsJSONEncoder, self).default(obj)
        File "/home/maubin/envs/reviewboard-dev/lib/python3.9/site-packages/django/core/serializers/json.py", line 124, in default
          return super(DjangoJSONEncoder, self).default(o)
        File "/home/maubin/.pyenv/versions/3.9.7/lib/python3.9/json/encoder.py", line 179, in default
          raise TypeError(f'Object of type {o.__class__.__name__} '
      TypeError: Object of type set is not JSON serializable
      -------------------- >> begin captured logging << --------------------
      reviewboard.admin.siteconfig: ERROR: Could not load siteconfig: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
      --------------------- >> end captured logging << ---------------------
      
    2. Looks like I had typoed it. Try changing {'.md', 'LessCss'} to {'.md': 'LessCss'} (, -> :)

    3. Oh oops, it does work now!

  11. 
      
maubin
maubin
Review request changed
Commits:
Summary ID Author
Add a setting for mapping file extensions to syntax highlighters for
diffs Adds a default mapping of file extensions to pygments lexers, which administrators may modify to define their own custom mapping. If a mapping is not defined for a certain file extension, we revert to the old behaviour of guessing which lexer to use given the filename and file contents.
810c268b04d766208982bd9cc5772fb472ab2e50 Michelle
Uses for else clause and .items() instead of .keys() for searching the
mapping
a5a683ce294e75b13c7c4b600ab1c6b7a2d9416d Michelle
Logs an error if a mapped lexer does not exist in Pygments
6da489bde81ca3eb9df84c3bb4e79425b18b7e45 Michelle
Removed trailing whitespace
8e396cb6f373d3336d411804d2973bb2f06680be Michelle
Properly imports mapping from settings.py
23ec2199b6b867851f9d8d8163a85e9ba9898bce Michelle
fixed string formatting for logged error
5785f25b427e1b18316db43ddcc98e02ba646892 Michelle
Made CUSTOM_PYGMENTS_LEXERS a class attribute (this makes it easier
to modify during testing)
eefc9d52856d30d61963a6d34ebc76102b33bf2b Michelle
took out print statement from testing
967ef3e281d39d468ef4d9c9f360f6a1e28087ec Michelle
Added unit tests for apply_pygment, testing that default
custom mapping works and that mapping to a non existant class logs an error
33139db632f2c36deb14cc9a493c4f33cb6754fe Michelle
Modified doc formatting and changed default test to instead
test if using a custom mapping overrides the guessing function to determine a lexer
224a9d9c751d4525937d3b4cae2f85a5f09d2e6a Michelle
fixed flake8 issues
e6eed2477b86a382140fbe0d215819ea4a17c8a9 Michelle
fixed one last flake8 indent issue
9b1f2c9a68e2161a9d67cb1064ec09063e8aeb50 Michelle
better logging message, fixed formatting issues, removed mapping attribute and left it as constant from settings
211c72f95930609b6655ad0f06f229df884d2d62 Michelle
fixed a couple flake8 issues
ad32f6009c25e8d555c19622857422236ee46f5f Michelle
moved mapping to siteconfig instead of settings.py
d7549e8107b35ceae203e9099b2a323bd7e89042 Michelle
initial form for mapping in diff viewer settings
09b9854b63f1c67e6684c300b7c9fd971a65d175 Michelle
added test for empty mapping
a36c2ee69209caa2053c1ea80431762a97cf9379 Michelle
create LexersMappingWidget and using in diff_settings.py for now to test
dfdc4468076bf6d899533efc6fb3c4b9f1088394 Michelle
create tests for lexers mapping widget
2d37e23bb79913a72a2e2c580d2023e73d7449c7 Michelle
testing listeditwidget with custom pygments lexers setting
38a50c83e212b487d4d62537b0fa64c9c6cbe2ff Michelle
add documentation and finalize code
8d4a6d7996c73bee56335d8f5ea55f16d4aee271 Michelle
no need for local site tests
19678b279aef9280842c45bebbf420098da9c9be Michelle
skip extension if its empty string since it would match any file
43a4b9f467132947149f2eb315abbcbd3cccbdb7 Michelle
fix some flake8 issues
df5ee812136e0eb9a94c4358a3288e26401d06fc Michelle
address review feedback
70eabb4e4983c7d39c06520389aaabde45d88b76 Michelle
address feedback
83df1297d190d6aa433b6a07e3acea09e499762f Michelle
fixed styling issues and uses siteconfig context
manager
8e3b46734371907145c91b86a75a4418ab332a6d Michelle
resolved merge conflicts from reviewboard-5.0.x
updates
4095f339e8f0d2f731c002a4188be81e0f6cd167 Michelle
Branch:
master
release-5.0.x

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
maubin
maubin
david
  1. 
      
  2. reviewboard/admin/form_widgets.py (Diff revision 19)
     
     
     
    Show all issues

    Add a blank line between these two.

  3. reviewboard/admin/form_widgets.py (Diff revision 19)
     
     
     
    Show all issues

    This needs to just be a single line. If we need more detail, we can add another paragraph.

  4. 
      
maubin
maubin
  1. 
      
  2. reviewboard/admin/form_widgets.py (Diff revision 20)
     
     
    Show all issues

    should I add a "Version added: 5.0.0" in here?

    1. Just "5.0" (no tiny versions for major releases).

  3. 
      
maubin
david
  1. 
      
  2. reviewboard/admin/form_widgets.py (Diff revision 21)
     
     
     
    Show all issues

    Since the entire class is added in 5.0, I don't think we need version specifiers for each method inside it. Here and below.

  3. 
      
maubin
david
  1. Ship It!
  2. 
      
maubin
Review request changed
Status:
Completed
Change Summary:
Pushed to release-5.0.x (5f1d486)