Ticket #3646 (closed enhancement: fixed)

Opened 5 years ago

Last modified 4 years ago

[with patch, positive review] Implement paren matching

Reported by: boothby Owned by: boothby
Priority: minor Milestone: sage-4.2
Component: notebook Keywords:
Cc: was, tclemans, yiqiang Work issues:
Report Upstream: Reviewers: John Palmieri
Authors: Tom Boothby Merged in: Sage 4.1.2.alpha4
Dependencies: Stopgaps:

Description (last modified by mpatel) (diff)

People have been complaining about a lack of paren matching in the notebook. Do something.

Discussions:  sage-devel,  sage-support.

Attachments

3646-paren-matching-all.patch Download (6.7 KB) - added by boothby 5 years ago.
3646-paren-matching-all.2.patch Download (6.8 KB) - added by boothby 4 years ago.
rebased against #6307
3646-silent.patch Download (3.2 KB) - added by boothby 4 years ago.
3646-paren-matching.patch Download (10.0 KB) - added by boothby 4 years ago.
replaces all previous
trac_3646-rebased.patch Download (6.4 KB) - added by jhpalmieri 4 years ago.
replaces all previous patches
trac_3646-rebased-v2.patch Download (6.4 KB) - added by mvngu 4 years ago.
replaces all previous patches + boothby's name as author
trac_3646-sagenb_paren_match.patch Download (6.3 KB) - added by mpatel 4 years ago.
Rebase v. #7196. Apply only this patch.

Change History

comment:1 Changed 5 years ago by boothby

  • Cc was, tclemans, yiqiang added
  • Summary changed from Implement paren matching to [with patch, needs review] Implement paren matching

comment:2 Changed 5 years ago by boothby

  • Milestone set to sage-3.0.6

comment:3 Changed 5 years ago by jhpalmieri

On my intel mac: works for me in Safari, and works in Firefox but noisily: it beeps whenever I hit ctrl-0 (whether it matches a parenthesis or not).

Changed 5 years ago by boothby

comment:4 Changed 5 years ago by boothby

I just discovered quilt! 4 patches all rolled into one.

comment:5 Changed 5 years ago by boothby

Oh, and this patch addresses jhpalmieri's issue.

comment:6 Changed 5 years ago by was

  • Summary changed from [with patch, needs review] Implement paren matching to [with patch, needs work] Implement paren matching

REVIEW:

  • Looks very good but does *not* address jhpalmieri's issue successfully. I can replicate the bing, which is OK, but pressing ctrl-] does not get recognized at all. I'm using a macbookpro.

Changed 4 years ago by boothby

rebased against #6307

Changed 4 years ago by boothby

comment:7 Changed 4 years ago by boothby

  • Summary changed from [with patch, needs work] Implement paren matching to [with patch, needs review] Implement paren matching

Ok, this really solves jhpalmieri's issue. This time, I tested it on a mac with firefox! I removed the ctrl-] keycode.

comment:8 Changed 4 years ago by jhpalmieri

  • Summary changed from [with patch, needs review] Implement paren matching to [with patch, needs work] Implement paren matching

If I create a cell with 

3+2)

put the cursor at the end and hit ctrl-0, the parenthesis gets replaced with "undefined". Is this the intended behavior? It doesn't seem ideal to me; maybe ctrl-0 should do nothing in this situation?

(Otherwise, it works well for me.)

comment:9 Changed 4 years ago by mpatel

  • Description modified (diff)

Changed 4 years ago by boothby

replaces all previous

comment:10 Changed 4 years ago by boothby

  • Summary changed from [with patch, needs work] Implement paren matching to [with patch, needs review] Implement paren matching

comment:11 Changed 4 years ago by jhpalmieri

  • Summary changed from [with patch, needs review] Implement paren matching to [with patch, positive review] Implement paren matching

This looks good to me.

comment:12 Changed 4 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Authors set to Tom Boothby

Oops, sorry, I forgot: the patch doesn't apply cleanly for me against 4.1.2.alpha2. I'm attaching a rebased patch, but all credit should still go to boothby.

Changed 4 years ago by jhpalmieri

replaces all previous patches

Changed 4 years ago by mvngu

replaces all previous patches + boothby's name as author

comment:13 Changed 4 years ago by mvngu

The patch trac_3646-rebased-v2.patch is the same as trac_3646-rebased.patch. The only difference is that v2 has the author name set to Tom Boothby.

comment:14 Changed 4 years ago by mvngu

  • Status changed from new to closed
  • Resolution set to fixed
  • Merged in set to Sage 4.1.2.alpha3

Merged trac_3646-rebased-v2.patch.

comment:15 Changed 4 years ago by mvngu

  • Merged in changed from Sage 4.1.2.alpha3 to Sage 4.1.2.alpha4

There is no 4.1.2.alpha3. Sage 4.1.2.alpha3 was William Stein's release for working on making the notebook a standalone package.

Changed 4 years ago by mpatel

Rebase v. #7196. Apply only this patch.

comment:16 Changed 4 years ago by mpatel

Patch Download rebased against SageNB's #7196.

comment:17 Changed 4 years ago by was

  • Milestone changed from sage-4.1.2 to sage-4.2

Merged into sagenb (for the sage-4.2 release cycle).

Note: See TracTickets for help on using tickets.