Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Rijnard van Tonder
    @rvantonder
    This message was deleted
    Whoops that link won't work it doesn't use staging.
    Ok sorry about this, but just go to this link and make it point to staging.comby.live instead of comby.live: bit.ly/3fPQM4z (I'll fix the link sharing here later)
    If you build from source you can also develop against this regex-supported version of comby, you can grab it from this branch: comby-tools/comby#162. The syntax works like I described, the only reason I haven't merged it yet is because there are some whitespace cases to fix.
    Sergey Kostyaev
    @s-kostyaev
    Thank you) I think I'll try this branch.
    Rijnard van Tonder
    @rvantonder
    Sounds good :-)
    Hopefully I can release this in the next week or two 🤞
    Sergey Kostyaev
    @s-kostyaev
    Will wait)
    Rijnard van Tonder
    @rvantonder
    Ok, I can ping you and I'll let you know. Otherwise, I can just release/merge soon and fix the missing parts after, as they are fairly specific and not so important to handle.
    Sergey Kostyaev
    @s-kostyaev
    As you wish) I already subscribed to this PR.
    Sergey Kostyaev
    @s-kostyaev
    @rvantonder , do I need some flag to use regex-hole? I can't make it work:
    %  comby -match-only 'func (:[name] :[type]) :[[method]](:[?args]) :[ret~[\w\s,()]*] {:[before]}' '' .go
    %  comby -match-only 'func (:[name] :[type]) :[[method]](:[?args]) :[ret] {:[before]}' '' .go
    /home/feofan/go/src/playground/at-2020-06-22-124608/snippet.go:25:func (v Vertex) Print() {\n    fmt.Printf("%+v\n", v)\n}\n\nfunc (v *Vertex) Scale(f float64) {\n    v.X = v.X * f\n    v.Y = v.Y * f\n    v.Z = v.Z * f\n}
    Also, it fail checks and I disable it to build this branch:
    ==> Starting build()...
    Nothing to do.
    ==> Starting check()...
         patdiff (internal) (exit 1)
    (cd _build/default && /home/feofan/.opam/4.09.1/bin/patdiff -keep-whitespace -location-style omake -unrefined test/common/test_pipeline.ml test/common/test_pipeline.ml.corrected)
    ------ test/common/test_pipeline.ml
    ++++++ test/common/test_pipeline.ml.corrected
    File "test/common/test_pipeline.ml", line 37, characters 0-1:
     |      ; substitute_in_place = true
     |      ; disable_substring_matching = false
     |      }
     |  ; output_printer = (fun _ -> ())
     |  ; interactive_review = None
     |  }
     |
     |let%expect_test "interactive_paths" =
     |  let _, count =
     |    let scheduler = Scheduler.create ~number_of_workers:0 () in
     |    Pipeline.with_scheduler scheduler ~f:(
     |      Pipeline.process_paths_for_interactive
     |        ~sequential:false
     |        ~f:(fun ~input: _ ~path:_ -> (None, 0)) [])
     |  in
     |  print_string (Format.sprintf "%d" count);
    -|  [%expect_exact {|0|}]
    +|  [%expect.unreachable]
    +|[@@expect.uncaught_exn {|
    +|  (* CR expect_test_collector: This test expectation appears to contain a backtrace.
    +|     This is strongly discouraged as backtraces are fragile.
    +|     Please change this test to not include a backtrace. *)
    +|
    +|  (Division_by_zero)
    +|  Raised at file "src/procs/hack_bucket.ml", line 40, characters 16-51
    +|  Called from file "src/procs/hack_bucket.ml", line 49, characters 14-41
    +|  Called from file "src/interface/scheduler.ml", line 59, characters 10-59
    +|  Called from file "lib/pipeline/pipeline.ml", line 268, characters 6-17
    +|  Called from file "lib/pipeline/pipeline.ml", line 260, characters 15-26
    +|  Called from file "lib/pipeline/pipeline.ml", line 260, characters 15-26
    +|  Called from file "test/common/test_pipeline.ml", line 31, characters 4-165
    +|  Called from file "collector/expect_test_collector.ml", line 225, characters 12-19 |}]
     |
     |let%expect_test "launch_editor" =
     |  let configuration =
     |    { configuration
     |      with interactive_review =
     |             Some
     |               { editor = "vim"
     |               ; default_is_accept = true
     |               }
     |    }
     |  in
     |  let result =
     |    try Pipeline.run matcher configuration; "passed"
     |    with _exc -> "Not a tty"
     |  in
     |  print_string result;
    make: *** [Makefile:28: test] Error 1
    ==> ERROR: A failure occurred in check().
        Aborting...
    Rijnard van Tonder
    @rvantonder

    @s-kostyaev you'll have to remove the space space between :[ret...] and {:[before]}, so that it is just :[ret...]{:[before]}. This is because otherwise comby will try match "space" then then the regex and then space again, which doesn't match the case where there is only one space. Sorry that's not intuitive, that's part of what I'm trying to make easier.

    For the crash: ah, I have to rebase the branch to make those tests pass. It should run OK without the passing checks though.

    Sergey Kostyaev
    @s-kostyaev
    I'll try, thanks.
    Sergey Kostyaev
    @s-kostyaev
    Thank you, @rvantonder ! It works like a charm!
    Vasilij Schneidermann
    @wasamasa
    Hello, having issues building this thing on Arch.
    I've tried building from source with the stock (4.10) and recommended (4.09) version of Ocaml. Stock build fails with an Ocaml syntax error, 4.09 build fails with an error from gcc. Now I've tried building https://aur.archlinux.org/packages/comby/ and get an error for Comby sources:
    ∗ installed shell.v0.12.0
    ∗ installed patdiff.git
    Done.
    
    <><> lwt.4.5.0 installed successfully <><><><><><><><><><><><><><><><><><><><><>
    => Lwt 5.0.0 will make some breaking changes in December 2019. See
         https://github.com/ocsigen/lwt/issues/584
    
    <><> bisect_ppx.git installed successfully ><><><><><><><><><><><><><><><><><><>
    => Bisect_ppx 2.0.0 has deprecated some command-line options. See
         https://github.com/aantron/bisect_ppx/releases/tag/2.0.0
    File "lib/rewriter/rewrite_template.ml", line 22, characters 4-12:
    22 |   | Ok label -> List.find_map label ~f:ident
             ^^^^^^^^
    Error: This pattern matches values of type
             ('a, 'b) Core._result = ('a, 'b) Stdlib.result
           but a pattern was expected which matches values of type
             consume:Angstrom.Consume.t ->
             (string option list, string) Stdlib.result
    make: *** [Makefile:10: release] Error 1
    ==> ERROR: A failure occurred in build().
        Aborting...
    Vasilij Schneidermann
    @wasamasa
    The main difference with this PKGBUILD is that it builds with 4.09.1 which fixed a bad interaction with gcc 10, so you might want to recommend that version instead of 4.09
    Vasilij Schneidermann
    @wasamasa
    It builds now after applying this patch: comby-tools/comby@995e786
    Vasilij Schneidermann
    @wasamasa
    One more stupid question: How do I run comby on a specific file only?
    If I use comby function '' test.js -o (to match the function keyword for test.js), it instead searches the current directory and fails on an unreadable subdirectory
    Vasilij Schneidermann
    @wasamasa
    Same behavior if I omit -o.
    Rijnard van Tonder
    @rvantonder
    @wasamasa to make sure you only run on a specific file or set of files, you can pipe using -stdin and -stdout options. For multiple files combine that with xargs. It should be the case that an absolute or complete relative path like ./path/to/test.js doesn't cause comby to traverse the directory, but in the case that's not working, the piping will definitely work.
    Vasilij Schneidermann
    @wasamasa
    Yeah, piping is an option but annoying. I've tried with a full path, but got the same error. Should I report a bug with a repro?
    Rijnard van Tonder
    @rvantonder
    @wasamasa yes please, if you can file an issue with a repro I can look into it :pray:
    P.D. Reiter
    @pdreiter
    Hi @rvantonder - I'm looking for an example comby template code (i.e. match/rewrite files) that matches the content of "Rewrite expressions" from the Advanced Usage in the Comby Docs.
    Rijnard van Tonder
    @rvantonder
    Ah, actually that one doesn't have a rewrite expression :-) I'll respond a bit later with more
    P.D. Reiter
    @pdreiter
    :+1:
    Vasilij Schneidermann
    @wasamasa
    It took me some time, but I've finally reported the bug with repro: comby-tools/comby#190
    Rijnard van Tonder
    @rvantonder
    Fixed in upstream. Thanks again for the report, makes it easy to check things. Will cut a release after I add a test (tomorrow or so).
    Vasilij Schneidermann
    @wasamasa
    An easier way of testing without sudo would be creating a subdirectory containing another matching line
    Lukas Werling
    @lluchs

    I'm trying to use comby for a replacement like this: GetCrew(plr, idx) to GetPlayer(plr)->GetCrew(idx). I came up with comby 'GetCrew(:[plr],:[? ]:[idx])' 'GetPlayer(:[plr])->GetCrew(:[idx])' and have two questions:

    1 - is :[? ] the best way to handle optional whitespace between arguments? The examples (e.g., the swap one on the installation page) just seem to assume that there is always a single space. Have you considered a way to strip whitespace in the replacement pattern?

    2 - The idx parameter is actually optional, and I'm not sure how to handle that with comby. So I also need to replace GetCrew(plr) with GetPlayer(plr)->GetCrew()

    Rijnard van Tonder
    @rvantonder

    Hey @lluchs :-) Good questions. For the first: since comby runs on the concrete syntax, the only way to specify an optional match in a single template is with the option you used :[? ]. Very recently, I've added regex support to comby, that you can use instead, which is perhaps more familiar and flexible for scanner/lexer kinds of matches. Unfortunately I haven't documented it on the site yet (soon!), but the basic syntax is :[thing~regex], where thing is an optional identifier name. Example on your code: https://bit.ly/315pe70. You'll need v0.18.1, I'm not sure that it is in brew yet, it may be. So, this is the best way I can see to do this, if you want to just use a single template. But:

    Have you considered a way to strip whitespace in the replacement pattern?

    If you can remove the constraint of specifying just a single template, you can strip whitespace using a rewrite rule. This example will strip the leading whitespace off of everything that matches <spaces>:[idx]: https://bit.ly/2Ycbdm2

    For question 2: There's always the possibility of doing this with two invocations. The single argument case would match something like GetCrew(:[[plr]]). There is a way to do everything in one invocation, but it's more complicated. So that said, it turned out to be a really interesting thing to try do with comby. Normally, rewrite rules can get you pretty far on their own, and that's what I tried at first: https://bit.ly/3aBSepX. The problem there is that it's not possible to assign a value to idx in a rule (and that's a need that came up in the past, but not often). It is something I may add, but it lead me to try solve this a different way.

    I came up with a way to use regex to match the first argument (assuming it is alphanumeric) , and then a rewrite rule to strip the leading commas/whitespace if it matches the idx part. It looks like this: https://bit.ly/3aDzKoU. Just expand the box to see the rules but here's a screenshot:

    Screen Shot 2020-08-18 at 11.22.28 AM.png
    Lukas Werling
    @lluchs
    Hi @rvantonder - wow, thanks for the detailed answer.
    Regex support works nicely (I downloaded the release for Linux), though the regexes might work better if they were a bit less greedy? I was trying :[~[^,]+] to match everything up to the first comma, but of course that then also matches the closing ) if there is no comma, and the whole match fails.
    Unfortunately, I cannot assume a purely alphanumeric first argument (it's often another function call), so a three-step replacement is probably the solution.
    Lukas Werling
    @lluchs
    Ah, nice - with a configuration file, I can do all three steps at once.
    But yeah, capturing values in a rewrite rule would be awesome for this use-case.
    Rijnard van Tonder
    @rvantonder

    but of course that then also matches the closing ) if there is no comma

    Yeah, that's unfortunately the main pitfall of introducing regex. And yep, I assume that if you exclude ) in the regex, like ~[^,)]+ it doesn't work for your use case because the first argument might be an expression like foo->bar() which will then also not (in general) match what you want.

    There is a way to implement the notion of "match a balanced expression but exclude <character set>, like comma", instead of "match a regex but exclude <character set>" which would address this. I'd need to introduce a syntax for that, and haven't thought about it yet :-)

    Ben Briggs
    @ben-eb
    Probably another vote for the above; I'd like to replace instances of Array.from(x).map(z) to Array.from(x, z) but not when Array.from(x, y).map(z) as that would result in Array.from(x, y, z) which would be incorrect (y and z would have to be combined through function composition). :slight_smile:
    Ben Briggs
    @ben-eb
    Maybe goes without saying but x, y and z could be any expression potentially
    Rijnard van Tonder
    @rvantonder

    Yeah, that makes a lot of sense. I've played around with something that'll match syntax for expressions like foo(x, y), so that Array.from(foo(x, y)) still matches despite the whitespace. The tricky part is also supporting something like:

    Array.from(function (...) {...})

    The function is one expression, but comby doesn't know about function being meaningful/delineating a keyword for the start of a function expression, so will just treat function as token, and the rest as structured blocks. You could use multiple templates here but obvi it's nice to have just hole syntax to match a JS expression (otherwise we wouldn't be talking about this :P). My thinking at this stage is to encode expression holes on a per-language basis so that this just works out of box.

    There's also the chance here that maybe you wouldn't want to do that replacement for something like function ... if it spans multiple lines. In which case it starts to make sense to define the syntax (subset) of expressions to match, possibly in a global form for all the patterns.
    Ben Briggs
    @ben-eb
    Yeah, I don't know how difficult that'd be to implement in a language agnostic way. Thanks for your insights!
    Rijnard van Tonder
    @rvantonder
    I think there's a way, it probably means handing over more user control/configuration over the parsing steps.
    Jason Keene
    @jasonkeene
    Hello, I just discovered Comby and am loving it! I'm looking for help trying to match the middle case in this example: http://bit.ly/33fiEL8
    It involves matching a sub expression that may or may not be nested at different levels of {} delimiters. Is there any way to match no matter how many levels the sub expression might be?
    Jason Keene
    @jasonkeene
    I was thinking I might be able to use the regex matching syntax to match { but I can't get it to work :/