PR: Make Pygments to work correctly with QSyntaxHighlighter #3491

ccordoba12 · 2016-10-02T08:54:30Z

Fixes #2122

Context:

The purpose of this PR is to finish @randyheydon's work started here: https://bitbucket.org/spyder-ide/spyderlib/pull-requests/103/
Right now we use Pygments to get tokens (and then highlight them) in a line by line basis. However, that's not how Pygments really works: Pygments needs to tokenize the whole file to correctly identify comments, strings, classes, etc.
This is why, for instance, multi-line comments in CSS files are not highlighted correctly in Spyder right now.
So we need to pass Pygments the entire file (instead of the current line) for tokenization, but in a way that doesn't introduce visible lags when working with big files (i.e. files with more than 1000 lines of code). @randyheydon already did the first part, and @goanpeca have done the second one.

TODO:

Move Pygments tokenization to a thread
Only apply it when the user stops typing

ccordoba12 · 2016-10-02T09:03:13Z

Note to self: This could work for task 2:

https://wiki.qt.io/Delay_action_to_wait_for_user_interaction

ccordoba12 · 2016-10-02T09:19:41Z

Also see this for how much to wait to decide if a user has stopped typing:

http://ux.stackexchange.com/a/38545

@randyheydon

This work was done by @randyheydon in https://bitbucket.org/spyder-ide/spyderlib/pull-requests/103 and now we're moving it to GitHub

goanpeca · 2016-10-04T20:44:02Z

@ccordoba12 could you provide some context on the description?

ccordoba12 · 2016-10-05T04:25:40Z

@goanpeca, sorry for being so brief :-) I updated the description to be more informative.

randyheydon · 2016-10-14T12:17:03Z

Thanks for carrying this on @ccordoba12. I don't think I'll have a chance to help with this, but I would like to make a quick note. The best way to fix this, I think, would be to change Pygments to support incremental lexing. In the past, I believed they would not accept such a change, since it's outside of their scope. However, I've since noticed that they have the same problem with the -s (streaming) option to the pygmentize script. It currently does the same thing as Spyder, by highlighting each line individually. Therefore, it should be possible to change Pygments to create an incremental highlighting mode that could be used by both pygments -s and Spyder.

I haven't looked into it too much, but I imagine this would be done with a new or modified method that takes the current lexer state as an argument, and includes the final lexer state as part of the returned data. That way, you could parse a line based on previous state, highlight that line, then parse the next line with the new state. Some of the individual language parsers would also need to be modified so that they split multi-line constructs into shorter constructs with intermediate states.

goanpeca · 2017-06-01T03:05:36Z

@ccordoba12 what is the status of this? I could add the generic threaded worker from the work I will add for the spyder-anaconda plugin. We could then use that worker to run things like what we want to accomplish here?

jitseniesen · 2017-06-01T12:30:24Z

@goanpeca Regarding this "generic threaded worker" you are working on, does the worker run on a different thread in the same process, or a different process?

I'm asking because for the unittest plugin, I need a worker in a different process which can continuously communicate the results back to spyder, so that the display can be updated in real time instead of having to wait until all tests are run.

goanpeca · 2017-06-01T15:26:03Z

@jitseniesen yes, I will have a generic worker for running python stuff (a method/func? that may be a long process) and a specific set of workers for a couple of tasks. A QProcess worker (that effectively runs a separate process and periodically signals (worker, stdout, stderr) so you can use it as well. I will add this code as generic stuff on spyder so any plugin can use this workers for long running processes.

jitseniesen · 2017-06-01T17:16:18Z

Excellent, I'll wait for it to appear so that I can use it instead of making my own. Perhaps we can also make the introspection stuff use your generic worker.

goanpeca · 2017-06-02T15:15:53Z

Perhaps we can also make the introspection stuff use your generic worker.

We are planning on using https://github.com/Microsoft/language-server-protocol, which already provides a generic framework for what we need, plus there is already a python implementation plus many other languages, so we would get completion, linting for free for all those languages

goanpeca · 2017-06-04T17:02:14Z

spyder/utils/workers.py

+        self._is_finished = True
+
+
+class ProcessWorker(QObject):


@jitseniesen this is the idea for the process worker, maybe you could take a look and see if it fits your needs.

@goanpeca Yes, I think that will work for me.

ccordoba12 · 2017-06-13T01:31:54Z

@goanpeca, how is this going? We need to merge this one as soon as we can, please.

ccordoba12 · 2017-06-13T02:31:10Z

spyder/utils/syntaxhighlighters.py

@@ -1074,33 +1075,81 @@ def __init__(self, parent, font=None, color_scheme=None):
        # Load Pygments' Lexer
        if self._lang_name is not None:
            self._lexer = get_lexer_by_name(self._lang_name)
+
+
+


Remove two blanks here and just leave one.

ccordoba12 · 2017-06-13T02:31:22Z

spyder/utils/syntaxhighlighters.py

+        self._allow_highlight = True
+
+    def make_charlist(self):
+        """"""


Missing docstring.

ccordoba12 · 2017-06-13T02:32:28Z

spyder/utils/syntaxhighlighters.py

+
+    def make_charlist(self):
+        """"""
+        def test_output(worker, output, error):


Why is this function called test_output? It would seem to imply that it's only used for tests.

ccordoba12 · 2017-06-13T02:33:59Z

spyder/widgets/sourcecode/codeeditor.py

+        # Highlight using Pygments highlighter timer
+        self.timer_syntax_highlight = QTimer(self)
+        self.timer_syntax_highlight.setSingleShot(True)
+        self.timer_syntax_highlight.setInterval(300)


Please add a comment to explain this 300 here.

ccordoba12 · 2017-06-13T16:19:42Z

spyder/utils/syntaxhighlighters.py

@@ -1052,6 +1052,7 @@ class PygmentsSH(BaseSH):
    # Store the language name and a ref to the lexer
    _lang_name = None
    _lexer = None
+    _charlist = []


This doesn't seem to be needed because _charlist is also initialized in __init__ below.

ccordoba12 · 2017-06-13T16:20:12Z

spyder/utils/syntaxhighlighters.py

+        # parsing
+        self._worker_manager = WorkerManager()
+
+        # Store the format for all the tokens after pygments parsing


pygments -> Pygments

ccordoba12 · 2017-06-13T16:20:28Z

spyder/utils/syntaxhighlighters.py

+        self._charlist = []
+
+        # Flag variable to avoid unnecessary highlights if the worker has not
+        # yet finish processing


finish -> finished

ccordoba12 · 2017-06-15T20:19:38Z

@goanpeca, do you want me to finish this one?

goanpeca · 2017-06-15T20:25:52Z

Oops I did not see the comments, giv me a sec

@ccordoba12 done!

ccordoba12 · 2017-06-16T03:37:31Z

Thanks a lot for helping me to finish this one @goanpeca!! Great work!

Fixes #2122 * Conflicts: - spyder/utils/syntaxhighlighters.py

ccordoba12 added this to the v3.1 milestone Oct 2, 2016

jitseniesen mentioned this pull request Oct 2, 2016

Refactor and unify source code analysis functionality #3493

Open

ccordoba12 changed the base branch from master to 3.x October 4, 2016 00:36

Make Pygments work correctly with QSyntaxHighlighter

6f3b385

This work was done by @randyheydon in https://bitbucket.org/spyder-ide/spyderlib/pull-requests/103 and now we're moving it to GitHub

ccordoba12 force-pushed the improve-pygments branch from a62dcb2 to 6f3b385 Compare October 4, 2016 02:37

goanpeca changed the title ~~Make Pygments work correctly with QSyntaxHighlighter~~ PR: Make Pygments work correctly with QSyntaxHighlighter Oct 26, 2016

ccordoba12 modified the milestones: v3.1, v3.1.1 Jan 6, 2017

goanpeca assigned ccordoba12 Jan 13, 2017

ccordoba12 modified the milestones: v3.1.1, v3.1.2, v3.1.3 Jan 18, 2017

ccordoba12 modified the milestones: v3.1.3, v3.2 Jan 27, 2017

ccordoba12 modified the milestones: v3.2, v3.3 Feb 14, 2017

goanpeca added 5 commits June 2, 2017 17:43

Add a threade worker manager for PythonWorkers and ProcessWorkers

215f6a2

Add timer to highlight whole file with pygments when using PygmentsSH

1b4eb2f

Add threaded worker on PygmentsSH to run whole file pygments

f2df6da

Merge with 3.x

9e56c23

Skip print check

3847cf8

goanpeca reviewed Jun 4, 2017

View reviewed changes

Add highlighting on startup

738f8e1

ccordoba12 commented Jun 13, 2017

View reviewed changes

Add comments and refresh highlighting on startup

299200d

ccordoba12 commented Jun 13, 2017

View reviewed changes

Fix typos in comments and update docstrings

93137c8

ccordoba12 changed the title ~~PR: Make Pygments work correctly with QSyntaxHighlighter~~ PR: Make Pygments to work correctly with QSyntaxHighlighter Jun 16, 2017

ccordoba12 merged commit d5bddbe into spyder-ide:3.x Jun 16, 2017

ccordoba12 deleted the improve-pygments branch June 16, 2017 03:38

ccordoba12 added a commit that referenced this pull request Jun 16, 2017

Merge from 3.x: PR #3491

c6c2286

Fixes #2122 * Conflicts: - spyder/utils/syntaxhighlighters.py

ccordoba12 mentioned this pull request Jul 17, 2017

Error when rendering reports on Windows spyder-ide/spyder-reports#27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Make Pygments to work correctly with QSyntaxHighlighter #3491

PR: Make Pygments to work correctly with QSyntaxHighlighter #3491

ccordoba12 commented Oct 2, 2016 •

edited

Loading

ccordoba12 commented Oct 2, 2016

ccordoba12 commented Oct 2, 2016

goanpeca commented Oct 4, 2016

ccordoba12 commented Oct 5, 2016

randyheydon commented Oct 14, 2016

goanpeca commented Jun 1, 2017

jitseniesen commented Jun 1, 2017

goanpeca commented Jun 1, 2017

jitseniesen commented Jun 1, 2017

goanpeca commented Jun 2, 2017

goanpeca Jun 4, 2017

jitseniesen Jun 5, 2017

ccordoba12 commented Jun 13, 2017

ccordoba12 Jun 13, 2017

ccordoba12 Jun 13, 2017

goanpeca Jun 13, 2017

ccordoba12 Jun 13, 2017

goanpeca Jun 13, 2017

ccordoba12 Jun 13, 2017

ccordoba12 Jun 13, 2017

ccordoba12 Jun 13, 2017

ccordoba12 Jun 13, 2017

ccordoba12 commented Jun 15, 2017

goanpeca commented Jun 15, 2017 •

edited

Loading

ccordoba12 commented Jun 16, 2017

PR: Make Pygments to work correctly with QSyntaxHighlighter #3491

PR: Make Pygments to work correctly with QSyntaxHighlighter #3491

Conversation

ccordoba12 commented Oct 2, 2016 • edited Loading

ccordoba12 commented Oct 2, 2016

ccordoba12 commented Oct 2, 2016

goanpeca commented Oct 4, 2016

ccordoba12 commented Oct 5, 2016

randyheydon commented Oct 14, 2016

goanpeca commented Jun 1, 2017

jitseniesen commented Jun 1, 2017

goanpeca commented Jun 1, 2017

jitseniesen commented Jun 1, 2017

goanpeca commented Jun 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Jun 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Jun 15, 2017

goanpeca commented Jun 15, 2017 • edited Loading

ccordoba12 commented Jun 16, 2017

ccordoba12 commented Oct 2, 2016 •

edited

Loading

goanpeca commented Jun 15, 2017 •

edited

Loading