Skip to content
This repository

make ENTER on a previous input field replace current input buffer #506

Closed
wants to merge 3 commits into from

3 participants

Martin Spacek Min RK Fernando Perez
Martin Spacek

I found this kind of functionality really handy in pycrust/pyshell under wx. To quickly replace the current input buffer with the contents of a previous input field, just click on the previous input field and hit enter. Then you can edit it before executing it.

Martin Spacek

It seems that for multi line inputs (with continuation prompts), this only copies the line that the cursor is currently at. Ideally, it should copy the whole set of lines with continuation prompts. Let's see if I can get that working...

Min RK
Owner
minrk commented

It might be helpful to just use the set_next_input machinery, used by %loadpy.

Martin Spacek

@MinRK: I looked at set_next_input(), but I didn't see anything particularly useful about it. Setting the input buffer is easy. The harder part was deciding whether the current line is an input line, and where the multi line input starts and stops.

I'm pretty happy with it now though.

Min RK
Owner
minrk commented

With set_next_input, all you need to do is know the prompt number for the current field - no need to try to manipulate the block, just:

  1. detect the id of the selected field
  2. execute get_ipython().set_next_input(In[id])

Also, try not to merge master into your branch. It's better to use rebase for a cleaner graph.

Martin Spacek

Ah, now I get it. I wasn't aware of the In and Out names in the shell. That certainly would be nicer. I'd only have to search up for the numbered prompt, instead of both up and down to find the boundaries of the input field. But in pure python mode, with no numbered prompts, I don't think that could work, could it? In doesn't seem to be bound. Also, in ipython mode, the user might for some reason bind something else to In, which would mess up retrieval of previous inputs for copying.

Sorry about merging in master. I'm still sort of winging it when it come to git. I'd be happy to create a new branch and pull request and reapply the net diff without all the messy commits.

Min RK
Owner
minrk commented

No need to create a new PR. You can rebase -i, and squash all your commits into a neater few if you feel like things are getting cluttered.

Or, if you feel like starting from scratch, you can just clobber your GitHub qt-copy-input branch with a 'push -f' (only if you are sure), and the new commits will be associated with this PR instead of the old ones.

Martin Spacek

All right, I started from scratch and overwrote my previous branch. The code is a bit cleaner now too.

IPython/frontend/qt/console/ipython_widget.py
@@ -227,11 +227,20 @@ class IPythonWidget(FrontendWidget):
227 227
         """
228 228
         text = self._control.textCursor().selection().toPlainText()
229 229
         if text:
230  
-            lines = map(transform_ipy_prompt, text.splitlines())
1
Min RK Owner
minrk added a note

With this change, it looks like IPythonWidget's copy() method is now identical to its parent's, so it can be removed entirely and just inherit, yes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Martin Spacek

Yes indeed. Good eye!

Martin Spacek

I just noticed that in pure Python mode, selecting a previous input field and hitting ENTER causes lockup and 100% cpu usage with no error. Not sure why. Maybe a recent commit in master?

Min RK
Owner
minrk commented

Nope - it's your downward search never terminating (print statements inside 'while True' reveals this).

You should break if the cursor doesn't move by adding an:

if cursor.position() == end (and start in the up search) to break if it's spinning its wheels at the end of the search.

That said, searching for valid input lines does not mean they are actually input lines. I added the break test eliminating your infinite loop, and discovered that your code was picking up too much with the pure prompts. I gave the following input:

a=1
b=2
print '>>> not input'
c=3

Each as a separate cell, and if I press enter at the end of any of those cells, they will all be included (including the output of the print statement) as the next input.

Martin Spacek

Ah, I think this is all because there isn't a blank line in between successive inputs in pure mode like there is in ipython mode. I guess I have to limit myself to finding exactly one non-continuation input prompt (>>>), not just any number of either type of input prompt.

As for the print '>>> not input' thing, with the current approach, I don't think that's avoidable. As you say, it looks like a valid input line to the user (and to the prompt regexp), and I don't think there's any way to distinguish it from a true input line. This holds for ipython mode as well, where In [1]: print 'In [2]: not input' causes problems, but in that case I think they may be avoidable.

Perhaps a tidier approach would be to start flagging lines (or line ranges) as input fields as they're generated. I don't suppose this already happens anywhere, does it? Could this info be saved as an attrib of the widget, maybe one of the _history* attribs in HistoryConsoleWidget?

Min RK
Owner
minrk commented

The print >>> noinput/print In[5]: foo is definitely avoidable - you can only have one non-continuation prompt in a block. The first one you see searching down means the end of the block. It doesn't, of course, protect against printing the continuation prompt.

But you are right, each input should be in an identifiable block that print statements can't mess with. The 'rich' widgets are actually HTML, I believe, so that should be quite easy, though I haven't the foggiest how to get access to that information. It should be straightforward to drop each block in a <div>, but I honestly have no idea how that works.

I also think it is perfectly appropriate for this to be a feature of the RichIPythonWidget, and thus not available in pure or plain mode (not that people actually use either, that I know of). That would make this easier, and it would also allow you to simply identify the execution ID of the current block, and actually call set_next_input(), which would be cleaner than trying to figure out the contents of the current block, etc.

Fernando Perez
Owner

Howdy, what's the status on this PR? Does it still need further work from @mspacek? I just want to make sure we complete review/merge of things before they diverge so much that the merge becomes a nightmare...

Min RK
Owner

Yes, this needs a bit more work, because as it is it can actually lock up the console by not properly detecting the edges of the window, and doesn't always grab a single input field, depending on line separations.

Fernando Perez
Owner

Thanks, @minrk. So I guess @mspacek, you should let us know if you'll have a chance to finish it up. We're happy to merge it when it's completed, but the longer it stays unmerged, the higher the chances something will get moved around in master causing a conflict. So just let us know if/when you get a chance to finish it up...

Martin Spacek

Hello! Sorry for not getting around to this. I would like to finish this up. I haven't been keeping up with IPython developments lately however, and I'll need to remind myself of the issues involved in getting this to work. I'll try and make this cleaner by limiting the feature to RichIPythonWidget and by using the set_next_input() as minrk suggested.

Fernando Perez
Owner

Great! And no worries, it's just that we don't want your work to go unused, so it's best to wrap it up before it goes stale.

Fernando Perez
Owner

@mspacek, just curious: do you think you'll have a chance to return to this one in the near future? It still merges cleanly but the risk of a conflict goes up the longer we let it linger :) If there's anything we can do to help you out, we'd be happy to.

Martin Spacek

Hi Fernando,

I don't know about the near future. I'm really bogged down trying to graduate. I think the patch needs to be completely redone anyway. I am still missing the feature every time I use qt ipython, so maybe that will annoy me enough one day to finally implement it correctly. Until then, perhaps this pull request should be closed down and turned into a feature request or something?

Fernando Perez
Owner

Martin, that sounds like a good plan; you should certainly focus on your studies first now (I say that as someone who made ipython his official thesis procrastination project ;)

I've opened #1049 and linked to this PR in there, so that it's easy to find. Whenever you find the time to hack on it, you can decide whether it's easier to continue with this PR (and if so, just reopen it) or to make a clean new branch.

Thanks again for your interest, and best of luck with graduation!

Fernando Perez fperez closed this
Damián Avila damianavila referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
2  IPython/core/inputsplitter.py
@@ -645,7 +645,7 @@ def transform_classic_prompt(line):
645 645
 _ipy_prompt_re = re.compile(r'^([ \t]*In \[\d+\]: |^[ \t]*\ \ \ \.\.\.+: )')
646 646
 
647 647
 def transform_ipy_prompt(line):
648  
-    """Handle inputs that start classic IPython prompt syntax."""
  648
+    """Handle inputs that start with classic IPython prompt syntax."""
649 649
 
650 650
     if not line or line.isspace():
651 651
         return line
50  IPython/frontend/qt/console/console_widget.py
@@ -665,6 +665,11 @@ def _prompt_finished_hook(self):
665 665
         """
666 666
         pass
667 667
 
  668
+    def _transform_prompt(self, line):
  669
+        """ Strip prompt from line.
  670
+        """
  671
+        pass
  672
+
668 673
     def _up_pressed(self, shift_modifier):
669 674
         """ Called when the up key is pressed. Returns whether to continue
670 675
             processing the event.
@@ -882,6 +887,15 @@ def _create_page_control(self):
882 887
         control.setVerticalScrollBarPolicy(QtCore.Qt.ScrollBarAlwaysOn)
883 888
         return control
884 889
 
  890
+    def _cursor_on_input_line(self, cursor):
  891
+        """ Return whether cursor is on an input line.
  892
+        """
  893
+        cursor.movePosition(QtGui.QTextCursor.StartOfBlock)
  894
+        cursor.movePosition(QtGui.QTextCursor.EndOfBlock,
  895
+                            QtGui.QTextCursor.KeepAnchor)
  896
+        line = cursor.selection().toPlainText()
  897
+        return self._transform_prompt(line) != line
  898
+
885 899
     def _event_filter_console_keypress(self, event):
886 900
         """ Filter key events for the underlying text widget to create a
887 901
             console-like interface.
@@ -948,6 +962,19 @@ def _event_filter_console_keypress(self, event):
948 962
                         self._control.moveCursor(QtGui.QTextCursor.End)
949 963
                         self._control.setTextCursor(cursor)
950 964
 
  965
+            else:
  966
+                # If current block is part of an input field, copy input field
  967
+                # to input buffer.
  968
+                start, end = self._get_input_field_range(cursor)
  969
+                if start != None and end != None:
  970
+                    cursor.setPosition(start)
  971
+                    cursor.setPosition(end, QtGui.QTextCursor.KeepAnchor)
  972
+                    lines = cursor.selection().toPlainText()
  973
+                    pastelines = []
  974
+                    for line in lines.splitlines():
  975
+                        pastelines.append(self._transform_prompt(line))
  976
+                    self._set_input_buffer('\n'.join(pastelines))
  977
+                    
951 978
         #------ Control/Cmd modifier -------------------------------------------
952 979
 
953 980
         elif ctrl_down:
@@ -1346,6 +1373,29 @@ def _get_input_buffer_cursor_prompt(self):
1346 1373
         else:
1347 1374
             return None
1348 1375
 
  1376
+    def _get_input_field_range(self, cursor):
  1377
+        """ Search up and down from cursor position for start and end
  1378
+            positions of an input field, made up of a contiguous set
  1379
+            of input and continuation lines. Return None values if
  1380
+            cursor is outside an input field.
  1381
+        """
  1382
+        orig = cursor.position()
  1383
+        start, end = None, None
  1384
+        while True: # search up
  1385
+            if not self._cursor_on_input_line(cursor):
  1386
+                break
  1387
+            cursor.movePosition(QtGui.QTextCursor.StartOfBlock)
  1388
+            start = cursor.position()
  1389
+            cursor.movePosition(QtGui.QTextCursor.PreviousBlock)
  1390
+        cursor.setPosition(orig) # back to original position
  1391
+        while True: # search down
  1392
+            if not self._cursor_on_input_line(cursor):
  1393
+                break
  1394
+            cursor.movePosition(QtGui.QTextCursor.EndOfBlock)
  1395
+            end = cursor.position()
  1396
+            cursor.movePosition(QtGui.QTextCursor.NextBlock)
  1397
+        return start, end
  1398
+
1349 1399
     def _get_prompt_cursor(self):
1350 1400
         """ Convenience method that returns a cursor for the prompt position.
1351 1401
         """
7  IPython/frontend/qt/console/frontend_widget.py
@@ -160,7 +160,7 @@ def copy(self):
160 160
         """
161 161
         text = self._control.textCursor().selection().toPlainText()
162 162
         if text:
163  
-            lines = map(transform_classic_prompt, text.splitlines())
  163
+            lines = map(self._transform_prompt, text.splitlines())
164 164
             text = '\n'.join(lines)
165 165
             QtGui.QApplication.clipboard().setText(text)
166 166
 
@@ -206,6 +206,11 @@ def _prompt_finished_hook(self):
206 206
         if not self._reading:
207 207
             self._highlighter.highlighting_on = False
208 208
 
  209
+    def _transform_prompt(self, line):
  210
+        """ Strip prompt from line.
  211
+        """
  212
+        return transform_classic_prompt(line)
  213
+
209 214
     def _tab_pressed(self):
210 215
         """ Called when the tab key is pressed. Returns whether to continue
211 216
             processing the event.
13  IPython/frontend/qt/console/ipython_widget.py
@@ -218,18 +218,13 @@ def _started_channels(self):
218 218
         self.kernel_manager.xreq_channel.history(hist_access_type='tail', n=1000)
219 219
 
220 220
     #---------------------------------------------------------------------------
221  
-    # 'ConsoleWidget' public interface
  221
+    # 'ConsoleWidget' abstract interface
222 222
     #---------------------------------------------------------------------------
223 223
 
224  
-    def copy(self):
225  
-        """ Copy the currently selected text to the clipboard, removing prompts
226  
-            if possible.
  224
+    def _transform_prompt(self, line):
  225
+        """ Strip prompt from line.
227 226
         """
228  
-        text = self._control.textCursor().selection().toPlainText()
229  
-        if text:
230  
-            lines = map(transform_ipy_prompt, text.splitlines())
231  
-            text = '\n'.join(lines)
232  
-            QtGui.QApplication.clipboard().setText(text)
  227
+        return transform_ipy_prompt(line)
233 228
 
234 229
     #---------------------------------------------------------------------------
235 230
     # 'FrontendWidget' public interface
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.