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

IPv6 addresses often mistaken as line/column numbers #146

Closed
mbunkus opened this issue Nov 11, 2022 · 4 comments
Closed

IPv6 addresses often mistaken as line/column numbers #146

mbunkus opened this issue Nov 11, 2022 · 4 comments

Comments

@mbunkus
Copy link
Contributor

mbunkus commented Nov 11, 2022

When I search for something & one of the result lines contains an IPv6 address, rg.el mistakes parts of them as the line & column number patters. For example, if I have the following whole line visible in *rg*:

terraform/lkbs/infrastructure.tf:226:    src_cidr = "2001:1640:141::/48"

and I press enter on it, Emacs asks me for the path to the file to visit with a nonsensical default:

Find this error in (default terraform/lkbs/infrastructure.tf:226:    src_cidr = "2001): …

The problem seems to be that the regular expressions used for building compilation-error-regexp-alist is too greedy.

I'm well aware that there's no 100% correct way of splitting rg's output (or maybe there is using the -0 arg…). Personally I'd very much like to be able to make the file name part non-greedy as I 100% never have file names with colons in them followed directly by numbers.

@dajva
Copy link
Owner

dajva commented Nov 16, 2022

Thanks for the report. Yes I think the -0 would probably work fine here. I guess we could use that for the --no-heading setup that you are using.

@dajva
Copy link
Owner

dajva commented Nov 17, 2022

After some testing I think -0 would be problematic mainly since it would be cumbersome to get rid of those ugly null chars in the output. TBH, I have not really crafted these regexps myself and never put much thought into it. I think it actually makes more sense to be restrictive with the kind of stuff that is allowed in file names, iow doing something like [^:]+ for matching the filename part. This seems to be what grep.el is doing anyway so should be fine to be in line with that.

@mbunkus
Copy link
Contributor Author

mbunkus commented Nov 17, 2022

Thank you for looking into it. I really appreciate it.

Yeah I figured having \0 in the output being bad, or at least requiring somewhat of a big-ish rewrite.

Like I said above I'd be perfectly fine with not allowing : as part of the file name, whatever regex you do it with. [^:]+ looks absolutely reasonable. I do know that : are allowed as part of file names on non-Windows systems, but then again I don't think this matters much wrt. to rg. We (as in: the collective users of rg) usually use it with code, with system configuration, maybe text archives. In each of those cases I've never seen : to be part of the file name. They're usually only used with system files, e.g. stuff in /tmp maybe.

The one exception that comes to mind is maildir directories.

Soooo… maybe making it configurable would be best? Or even toggle-able from the *rg* buffers?

dajva added a commit that referenced this issue Nov 20, 2022
This is causing troubles with line:column like patterns in files
gaining traction from ipv6 adoption. Let's just parse the filename up
until the first ':' here to improve the situation.
@dajva
Copy link
Owner

dajva commented Nov 20, 2022

Pushed a fix now without configuration possibilities. Let's add that if people request it.

@dajva dajva closed this as completed Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants