Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add qlik lexer #1925

Merged
merged 9 commits into from Mar 4, 2022
Merged

Add qlik lexer #1925

merged 9 commits into from Mar 4, 2022

Conversation

GDownAdviseInc
Copy link
Contributor

Created a lexer to support qlik scripting language.

Anteru
Anteru previously requested changes Oct 23, 2021
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jeanas jeanas left a comment

Choose a reason for hiding this comment

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

Please add Qlik in doc/languages.rst.

pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
@jeanas
Copy link
Contributor

jeanas commented Mar 3, 2022

@GDownAdviseInc The review comments are marked resolved, but I don't see the updates, did you forget to push them?

@GDownAdviseInc
Copy link
Contributor Author

@jean-abou-samra I'm still working on getting it tested as I need to download xcode to be able to use make etc but it's 10GB and taking a long time, so I haven't yet pushed it. I will get it there asap and let you know.

@GDownAdviseInc
Copy link
Contributor Author

@jean-abou-samra Thanks for looking into all of this and all your suggestions above. I believe I have now made all the changes required in the commits I've just done. I have tried to run make test, but I couldn't get it to run with pyenv. I was able to run pytest on it though, and there were two errors, but I don't believe they are related to the changes I have made (Although let me know if they are and I can try and fix it).

> pytest
======================================================================================== test session starts ========================================================================================
platform darwin -- Python 3.9.1, pytest-7.0.1, pluggy-1.0.0
rootdir: /Users/gemmadown/Documents/pygments, configfile: pytest.ini
collected 3796 items / 2 errors / 3794 selected                                                                                                                                                     

============================================================================================== ERRORS ===============================================================================================
__________________________________________________________________ ERROR collecting tests/test_html_formatter_linenos_elements.py ___________________________________________________________________
ImportError while importing test module '/Users/gemmadown/Documents/pygments/tests/test_html_formatter_linenos_elements.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../.pyenv/versions/3.9.1/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_html_formatter_linenos_elements.py:9: in <module>
    from .support import structural_diff
tests/support/structural_diff.py:1: in <module>
    import lxml.html
E   ModuleNotFoundError: No module named 'lxml'
_________________________________________________________________________ ERROR collecting tests/contrast/test_contrasts.py _________________________________________________________________________
ImportError while importing test module '/Users/gemmadown/Documents/pygments/tests/contrast/test_contrasts.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../.pyenv/versions/3.9.1/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/contrast/test_contrasts.py:18: in <module>
    import wcag_contrast_ratio
E   ModuleNotFoundError: No module named 'wcag_contrast_ratio'
====================================================================================== short test summary info ======================================================================================
ERROR tests/test_html_formatter_linenos_elements.py
ERROR tests/contrast/test_contrasts.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================================================= 2 errors in 6.33s =========================================================================================

@jeanas
Copy link
Contributor

jeanas commented Mar 3, 2022

You are missing packages required for testing. As Python projects often do, we provide a file requirements.txt at the root of the repository, containing the list of packages needed to develop Pygments. Try

python -m pip install --user -r requirements.txt

@jeanas
Copy link
Contributor

jeanas commented Mar 3, 2022

CI is failing on Qlik because the token output needs updating. Please do

pytest --update-goldens tests/examplefiles/qlik

and commit the result (after checking that it looks good).

@GDownAdviseInc
Copy link
Contributor Author

GDownAdviseInc commented Mar 3, 2022

@jean-abou-samra Can't believe I didn't reinstall from requirements! 🤦 Clearly been a long day. Thanks!

I've tried running pytest update-goldens, but getting an error. It's the same error I get when attempting to run pytest itself. I'm not familiar enough with this library at all to work out if this is something in my python set up (using pyenv virtualenv) or something else:

pytest --update-goldens tests/examplefiles/qlik
======================================================================================== test session starts ========================================================================================
platform darwin -- Python 3.9.1, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
Using --randomly-seed=3001490038
rootdir: /Users/gemmadown/Documents/pygments, configfile: pytest.ini
plugins: randomly-3.10.1, cov-3.0.0
collected 0 items / 1 error                                                                                                                                                                         

============================================================================================== ERRORS ===============================================================================================
___________________________________________________________________________________ ERROR collecting test session ___________________________________________________________________________________
../../.pyenv/versions/3.9.1/envs/pygments/lib/python3.9/site-packages/pluggy/_hooks.py:265: in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
../../.pyenv/versions/3.9.1/envs/pygments/lib/python3.9/site-packages/pluggy/_manager.py:80: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
tests/examplefiles/conftest.py:27: in pytest_collect_file
    return LexerTestFile.from_parent(parent, path=pathlib.Path(path))
E   TypeError: from_parent() missing 1 required keyword-only argument: 'fspath'
====================================================================================== short test summary info ======================================================================================
ERROR  - TypeError: from_parent() missing 1 required keyword-only argument: 'fspath'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================================================= 1 error in 0.25s ==========================================================================================

Thanks for your help with all of this - sorry for my inexperience with pygments.

@jeanas
Copy link
Contributor

jeanas commented Mar 3, 2022

That's on our end -- you need to upgrade pytest to version 7. I've opened #2080 so installing from requirements.txt will always give you a good environment.

Copy link
Contributor

@jeanas jeanas left a comment

Choose a reason for hiding this comment

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

A few more comments and we should be good to go.

pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
@jeanas
Copy link
Contributor

jeanas commented Mar 3, 2022

Thanks for the update. There are make check failures in the CI, could you take a look? It seems that three things need to be done:

  • Remove extra ~ in module name underlines in the docstrings,
  • Bump copyright to "2006-2022",
  • Import the names you need explicitly instead of from pygments.token import *.

pygments/lexers/qlik.py Outdated Show resolved Hide resolved
pygments/lexers/qlik.py Outdated Show resolved Hide resolved
@GDownAdviseInc
Copy link
Contributor Author

Thanks @jean-abou-samra - hopefully changes have been made. It's passing pytest and make check locally 🤞

pygments/lexers/qlik.py Outdated Show resolved Hide resolved
@jeanas jeanas dismissed Anteru’s stale review March 4, 2022 09:00

The requested changes have been made.

@jeanas jeanas merged commit d88a702 into pygments:master Mar 4, 2022
@jeanas
Copy link
Contributor

jeanas commented Mar 4, 2022

Thanks for your patience. Merged now.

@GDownAdviseInc
Copy link
Contributor Author

Thanks for all your help and patience @jean-abou-samra - I really do appreciate it.

@Anteru Anteru self-assigned this Mar 4, 2022
@Anteru Anteru added this to the 2.12.0 milestone Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants