astsearch icon indicating copy to clipboard operation
astsearch copied to clipboard

Add --print-multiline flag on Py3.8+.

Open anntzer opened this issue 4 years ago • 5 comments

Py3.8+ provides Node.end_lineno which is exactly what's needed for the feature. Test e.g. with astsearch -m 'if ??: ??'.

Closes #6.

I don't have a strong opinion as to how to name the option.

anntzer avatar Apr 27 '21 09:04 anntzer

Thanks!

What do you think about printing multiline matches by default, instead of requiring an extra option? It seems like a good idea if each match is just a few lines, and a bad one if it's 100 lines (especially if matches overlap!).

Maybe a neater way to control it is to have a configurable threshold for how many lines to print per match, and do some sort of ellipsis for longer matches:

  34│    if path.endswith('.so'):
    └ <15 lines in match>

Then you could re-run with --max-lines 15 if you wanted to see the full contents of that match.

takluyver avatar Aug 08 '21 09:08 takluyver

That seems fine to me, but note that this is only available on Py3.8, so you'll have to choose between restricting astsearch to only work on py3.8, or having different default behaviors on py<3.8 (print single line) and >=3.8 (print whole match). (As the PR is currently written, the effect is that the -m flag is only present on py3.8+.) I don't have a strong opinion either way, so you get to pick.

anntzer avatar Aug 16 '21 05:08 anntzer

Kindly bumping?

anntzer avatar May 12 '22 09:05 anntzer

Sorry for the long delay!

In this case, I think the best option is to let the behaviour differ by the version of Python in use, so newer versions of Python get the (hopefully) better feature, and older versions keep the current output. I'm not quite ready to drop 3.7 entirely yet (we're still using it at work), but I'm OK with requiring newer versions for improvements.

takluyver avatar Jun 05 '22 11:06 takluyver

OK, I implemented and pushed something close to your suggestion in https://github.com/takluyver/astsearch/pull/10#issuecomment-894772114 (except that exceedingly-long matches are still printed but elided after max_lines lines -- I think this behavior makes more sense, but don't feel strongly about it either).

anntzer avatar Jun 10 '22 16:06 anntzer

I'm interested in a slight variation on the output have all lines be:

<file>:<linenumber> <code>

Reason being the above format is often recognized by text editor to jump to line.

I'm happy to send a PR that encompass both my and and @anntzer changes, and @takluyver if you don't want to work on publishing a new version I'm happy to do so if you give me publish rights on PyPI.

Carreau avatar Jun 12 '23 11:06 Carreau

Thanks, and sorry again about the long delay in looking at this. I'm going to merge this PR as is now; I think it's a clear improvement.

@Carreau feel free to do what you suggest in a separate PR. Fitting with common editor conventions makes sense. It should obviously be behind an option, though, because I don't think that's generally the best format for human eyes.

Other miscellaneous ideas about presenting output:

  • Use the box drawing vertical line instead of pipe | to make the margins look neater?
  • How should overlapping matches be shown? E.g. if I search for 'for ? in range(?): ??' and the code has nested for i and for j loops, at present the inner loop is just shown twice. Maybe that's OK? Or maybe we can find a neater way?
  • Show the code with syntax highlighting? Pygments is my go to library for this. Might need some care in case a snippet starts/ends in the middle of a multi-line string.
  • Option for JSON output?

takluyver avatar Oct 25 '23 12:10 takluyver

How should overlapping matches be shown?

Good point. I guess a proper approach may be to first detect overlapping matches (this can probably be done in a "sliding window" approach, keeping track of all not-yet-exited blocks, checking if the next match overlaps with any of them, and if the next match "exited" a block then remove the block from the track list), then printing out the entire overlapping block all at once and replacing the left pipe/vertical line with > (or unicode arrow or whatnot) at the relevant lines.

anntzer avatar Oct 25 '23 12:10 anntzer