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

Unmatched Curly Quotes reporting matched quotes. #326

Closed
okrick opened this issue Jul 12, 2024 · 6 comments · Fixed by #362
Closed

Unmatched Curly Quotes reporting matched quotes. #326

okrick opened this issue Jul 12, 2024 · 6 comments · Fixed by #362
Labels
core feature Required for basic PPing

Comments

@okrick
Copy link

okrick commented Jul 12, 2024

  1. Tools=>Unmatched=>Curly Quotes
  2. Unmatched Curly Quotes shows:
355.0: Unmatched “
357.25: Unmatched ”

When using GG1 with the "Highlight Surrounding Quotes and Brackets" feature enabled, the system identifies the quotes on these two lines as a matching pair and highlights them in green. This occurs even though there's another set of double quotes within this section.

negro-src.zip

@tangledhelix
Copy link
Collaborator

@windymilla Since I was working on #42, which would be related to this issue, I went to look for the algorithm, and I realized I couldn't find it in the GG1 code. Do you know which layer implements it?

That is, I can find references to the function that does this highlight, but I can't find the definition. I would not have thought this is part of Tcl/Tk itself but... is it? What is TextEdit.pm? That's not part of the GG1 codebase either...

❯ grep -R HighlightSinglePairBracketingCursor .
./lib/Guiguts/Highlight.pm:# Calls to HighlightSinglePairBracketingCursor adapted from TextEdit.pm
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor( '(', ')', '[()]', 'CURSOR_HIGHLIGHT_PARENS',
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor( '{', '}', '[{}]', 'CURSOR_HIGHLIGHT_CURLIES',
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor( '[', ']', '[][]', 'CURSOR_HIGHLIGHT_BRACES',
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor(
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor(
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor(
./lib/Guiguts/Highlight.pm:    $textwindow->HighlightSinglePairBracketingCursor(

@tangledhelix
Copy link
Collaborator

I think I found it... Tk::TextEdit (line 432):

https://metacpan.org/release/SREZIC/Tk-804.036/source/Tk/TextEdit.pm

@windymilla
Copy link
Collaborator

@tangledhelix - yes, that's correct. As you discovered later, Perl/Tk does have that function in the TextEdit widget, which the GG1 main text widget inherits from. Porting it from the Perl version to Python would be one way of getting the functionality.
Although misc_tools.py:unmatched_markup_check() in GG2 is doing a similar job, it would probably not be worth trying to force it to also handle this case too. It's possible it might have something that could be useful for your port though.

@windymilla
Copy link
Collaborator

  1. Tools=>Unmatched=>Curly Quotes
  2. Unmatched Curly Quotes shows:
355.0: Unmatched “
357.25: Unmatched ”

When using GG1 with the "Highlight Surrounding Quotes and Brackets" feature enabled, the system identifies the quotes on these two lines as a matching pair and highlights them in green. This occurs even though there's another set of double quotes within this section.

@okrick - there is a compromise that needs to be made on whether to permit match items to be nested. If you don't allow it, which at the moment the Unmatched Curly Quotes does not, then you can get an false report, like the one you quote. However, if you do allow it, it can be harder to find out where the actual error is, and theoretically two errors are more likely to cancel each other out, so not get reported at all, e.g. (could be much more widely spaced than this contrived example, where the second quote is an open when it should be a close, and the third is a close when it should be an open).

“Now, we have got to figure“[**wrong quote], he asked, ”[**wrong quote]how much does it amount to?”

If on balance, you think it would be better to permit nesting for matching quotes, I can easily make that change. Since GG1 doesn't have the same check, there's no need for us to match a GG1 feature, so we can just choose whichever would be most helpful.

@okrick
Copy link
Author

okrick commented Jul 15, 2024

The ability to configure nesting within the report would be valuable. Some users might prefer seeing everything, including nested issues, while others might find it helpful to exclude them for better readability.
The current report format can be overwhelming, especially with potentially hundreds of lines, most of which might be false positives. Clicking through each line is a time-consuming task.


The Text -> Check Orphaned Brackets function in GG1 offers a superior user experience. It efficiently navigates between potential errors, eliminating the need to click line by line through a lengthy list. This makes it my preferred method for handling such issues. I'm a little upset that we appear to be dropping this function for a kludge.

To understand user sentiment towards GG1's "pptxt" function, a poll could be helpful. This would determine how many users actually rely on it.
Since I primarily use Text -> Check Orphaned Brackets, it seems beneficial to explore if replicating that functionality would be a better solution than the current report, especially if "pptxt" usage is low.

@windymilla
Copy link
Collaborator

Thanks @okrick - I can look to see if making nesting configurable would be easy to add.

Regarding Check Orphaned Brackets, don't be upset about the feature being dropped - no decision has been made to drop it!

There are a lot of features in GG1, and we have been working to add either identical or similar features to GG2. I can't guarantee that all your favorites from GG1 will make it into GG2, but we're obviously aiming for the system to work in an efficient way to help the user find problems.

The online pptext program that is part of the PPWB tools, is the tool that is used the most from that collection, about 400 to 500 times per month (according to Linda's monthly GM report). So, some people definitely find that format helpful, as well as the "lead you through" style of Check Orphaned Brackets.

It's helpful to get feedback to steer the development - you are very much at the leading edge, using GG2 for real work, which I really appreciate. I hope as GG2 gets some more of the basic features, that we will be able to broaden the base of users trying it out, so we get a good representative sample of feedback from people who use GG in a variety of ways.

windymilla added a commit to windymilla/guiguts-py that referenced this issue Aug 2, 2024
For bracket and curly quote unmatched checks, there is
now a checkbutton that allows the user to control
whether nesting is permitted, e.g.
`[Footnote: [**proofer note] text of footnote]`

User can change the setting and re-run.

Fixes DistributedProofreaders#326
windymilla added a commit to windymilla/guiguts-py that referenced this issue Aug 4, 2024
For bracket and curly quote unmatched checks, there is
now a checkbutton that allows the user to control
whether nesting is permitted, e.g.
`[Footnote: [**proofer note] text of footnote]`

User can change the setting and re-run.

Fixes DistributedProofreaders#326
@windymilla windymilla added the core feature Required for basic PPing label Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core feature Required for basic PPing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants