Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Smarter line clipping in Find in Files #9743

Merged
merged 2 commits into from
Jan 23, 2015

Conversation

marcelgerber
Copy link
Contributor

For #9659 (at least part of it).
Best to explain with a before/after:
Before:
image

After:
image

So there are three ways text can clip:

  • If endCh (last char of match) is < 200, the line is simply displayed and clipped after 200 chars
  • If the match is longer than 200 chars, the match is displayed, highlighted and clipped after 200 chars (may not be perfect, but you'll probably only get into this state with RegExps)
  • Else, the code tries to center the match in the displayed line (and, of course, it highlights the match)

@prafulVaishnav prafulVaishnav self-assigned this Jan 23, 2015
prafulVaishnav added a commit that referenced this pull request Jan 23, 2015
@prafulVaishnav prafulVaishnav merged commit b2df944 into adobe:master Jan 23, 2015
@marcelgerber marcelgerber deleted the find-in-match-highlight branch January 23, 2015 05:35

matches.push({
start: {line: lineNum, ch: ch},
end: {line: lineNum + numMatchedLines - 1, ch: (numMatchedLines === 1 ? ch + totalMatchLength : lastLineLength)},
end: {line: lineNum + numMatchedLines - 1, ch: endCh},
Copy link
Member

Choose a reason for hiding this comment

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

@marcelgerber I think this might run into some bugs with matches that span multiple lines. The code here still assumes the start ch is on the first line of the match and the end ch is on the last, which might be incorrect depending on which clipping case you fall into. This was already somewhat buggy before your change, so it may not be an urgent issue... but just fyi. In case you feel like doing any more work on this code :-)

Also, might not hurt to add some clipping tests to FindInFiles-test at some point.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, that comment should be attached to the line.substr()/.substring() calls, not the end: initializer here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Multi-line results are automatically being trimmed, so this shouldn't be a problem.
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants