search

beyondgrep / ack3

ack is a grep-like search tool optimized for source code.
https://beyondgrep.com/
Other
690 stars 66 forks source link

sane headless operation #103

Open n1vux opened 6 years ago

n1vux commented 6 years ago

re FAQ and --nofilter on https://github.com/beyondgrep/ack2/issues/649 -- I agree this is an ack2 FAQ candidate.

but for ack3, I think we should make it work headless: explicit file(s) or directory(s) in @ARGV is sufficient user intent that they aren't intending to offer a pipe on STDIN (and they can say - if they intend to mix files and pipes), so I maintain least surprise is for ack in crontab to work same as on commandline. --nofilter doesn't sound like --work-in-crontab, hardly intuitive. At this point in 3.0, we aren't tied to not-breaking, so we can do the right thing. A better error and a better FAQ isn't much help when there's no TTY, it may be hard to see STDERR. Saying use grep for non-interactive isn't good enough. We're beyond-grep in multiple ways for non-interactive, non-highlight uses. ack --type=$whatever $badword $dir && handle_errors() is not unreasonable and may be much simpler than equivalent grep if $whatever is complex.

petdance commented 6 years ago

Isn't the crontab issue really a shell script issue?

Also, we have to remember Windows.

petdance commented 6 years ago

We need to look at the history of this issue before we do anything. This has come up annually or so since the dawn of ack.

n1vux commented 6 years ago

Isn't the crontab issue really a shell script issue?

No. Yes, there are plenty of Shell issues with crontabe whereby expecting what works in login shell to work won't (thin secure environment so no paths etc) But an explicit path to one-file-ack and all needed libs in system perl (using built-in lib paths) will not hit those issues. $path/ack --help and $path/ack version and echo foo | $path/ack foo all work in crontab just fine.
If those work, ack --txt badword /tmp && fixit.sh should work DWIM .

Also, we have to remember Windows.

Since neither crontab nor /dev/TTY appear on windows, I agree the fix to headless shouldn't break windows. if whatever code is forcing it to --filter mode despite explicit @ARGV suppling files/dirs beyond the options and mandatory pattern is to make WIN work, perhaps it needs to be protected by an IF (OS) {}

petdance commented 6 years ago

But an explicit path to one-file-ack

I'm talking about the TTY detection, not libs.

n1vux commented 6 years ago

We need to look at the history of this issue before we do anything.

Agree, review of history is a good thing. For 3.0 we have a golden opportunity to fix what is not fixable due to ack2.0 installed base compatibility.

This has come up annually or so since the dawn of ack. Which is a clue that we have a DWIM failure. or a conflict of two DWIMs.

In the rest of the *nix infrastructure, if one expects to read STDIN pipe despite listing files on CLI, one includes - in the position desired to read STDIN

n1vux commented 6 years ago

I'm talking about the TTY detection, not libs.

How is TTY detection a shell thing?

n1vux commented 6 years ago

Why does nohup not provide equal breakage?

rm nohup.txt
/usr/bin/nohup ack --type=csv automotive CC*.csv &
sleep 5
cat nohup.txt

looks fine. Apparently nohup only fiddles FD [012] and SIGHUP, but doesn't change /dev/TTY

n1vux commented 6 years ago

History.

Ack1 (all CLOSED)

Ack2

Ack3

petdance commented 6 years ago

How is TTY detection a shell thing?

All I can tell you is that there have been problems with filter vs. direct detection in shell scripts. I don't know more specifics.

n1vux commented 6 years ago

every other code I've seen that takes input from both listed files or STDIN, (a) does the filter DWIM based on if there are excess @ARGV or not, (b) accepted - as "read STDIN in turn here", (c) and ignored TTY.

Are we being too smart where others succeed by trusting the arguments? Because we need to know if interactive to know whether to highlight DWIM or not?
Piped input does not mean not highlighting output. Buffered-ness out STDOUT might be more appropriate clue to highlight DWIM?

hugopeixoto commented 1 year ago

I just ran into this issue in Gitlab CI. Assuming problems related to "interactiveness", I switched to grep and it worked.

Trying to understand why, I searched the man page for interactive and standard input/stdin, but couldn't find anything. I'm not sure if the --[no]filter option text is enough to highlight the issue.

I used strace and only saw calls to fstat(STDIN). Looking at the codebase, I traced it back to $is_filter_mode = -p STDIN; and its usage, which explains the problem: the first thing ack does is check if stdin is a pipe, and if so, it extracts an argv pattern, ignores argv paths and assumes that it is to filter text from stdin. This also causes echo foo | ack -f to also fail: if stdin is a pipe, it tries to extract an argv pattern that isn't there.

I agree that explicitly giving ARGV path to scan should trump STDIN. This doesn't need to affect ack's output, only its input (any TTY detection doesn't need to be touched). If changing this behavior is not an option, ack could at least:

For reference, here's a small test that shows the different behaviors using docker:

$ cat poop.sh
#!/usr/bin/env sh
apk add --update --no-cache ack file;
file /proc/self/fd/0
ack -f

$ BLABLA="--rm -w /src -v $(pwd):/src alpine:latest ./poop.sh"

$ docker run $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/null

$ docker run -i $BLABLA | grep proc
/proc/self/fd/0: symbolic link to pipe:[12640013]
ack: No regular expression found.

$ docker run -t $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/pts/0

$ docker run -it $BLABLA | grep proc
/proc/self/fd/0: symbolic link to /dev/pts/0