[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 This was rebased on master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/385 feel free to move ahead with the rebase and i can then get this merged. I think that the zeros can go now that we have the dots which i like the more each time i see it. thanks - this was a really nice contribution. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 @spmallette I can rebase on latest master. Just let me know. Do you think there is any point in keeping the leading zeroes? ``` gremlin> 1+ ..1> 2+ ..2> 3+ ..3> 4 ``` Alt prompt: ``` g> 1+ 1> 2+ 2> 3+ 3> 4 ==>10 ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 OK. And when this is merged with the color preferences PR, it will be adjusted and left-filled with `.` for the input.prompt length, but no less than `000>`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/385 i prefer the `>` line up though - fwiw, of the above that i tinkered with I liked: ```text gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ``` the best --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 I would only be against `===>` because that looks like the result prompt. I would probably lean towards `001>`. Maybe: ``` gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/385 I would think it should use the same color as the `input.prompt.color`. I'm not sure it needs to be configurable. Just wondering if there was a better presentation. For some reason, spaces look off to me, but I'll go with whatever people with stronger opinions are up for. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 Once the color preference support is added in, I can make this a preference. The line number can be '%n' so that it can be set with `:set multiline.prompt "%n>"`. Should it have its own color or just use input.prompt.color? So maybe wait until that merge happens first? Or I can merge this in with color preference PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/385 I tested this out manually - works nicely. I don't really like the spaces between line number and `>`: ```text gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 ``` wonder if anyone else feels the same. i tried filling them with other things for fun to see if anything looked better: ```text gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 gremlin> 1 + 001> 1 + 002> 1 + 003> 1 ==>4 ``` Anyone feel the same? Otherwise: VOTE +1 from me. Note that this PR could use a CHANGELOG entry and upgrade docs in the users section as i think it's a nice feature to announce. If @robertdale has time to amend this PR with that change, that would be cool, if not, whoever merges should tack that in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/385 I agree with @dkuppitz . Lets make another ticket about deprecating and removing `IdentityRemovalStrategy`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/385 We should ask ourselves if `IdentityRemovalStrategy` is really as great as it should be. We need to chain dozens of `identity()`s to prove that `IdentityRemovalStrategy` makes the traversal execution faster. This looks way too artificial to me. Why would anybody end up with dozens of `identity()`s? IMHO we should either: * deprecate the strategy and remove it completely in a later release or * remove it from the default strategies and only run performance tests for more than 5 chained `identity()`s I think I'd prefer the former as I really can't see any real use cases where this strategy would be advantageous. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/385 I lowered the threshold for passing on that test even lower in master yesterday after i saw travis has been failing over it lately. hopefully it is low enough now - if not @dkuppitz we may need to do something else with: ```text IdentityRemovalStrategyTest$PerformanceTest>TraversalStrategyPerformanceTest.shouldBeFaster:118 null ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #385: TINKERPOP-1285 added multi-line line number support
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/385 This is failing due to some random error, not the code change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
