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

Revset syntax error on .^:: #612

Closed
francois opened this issue Apr 19, 2023 · 1 comment
Closed

Revset syntax error on .^:: #612

francois opened this issue Apr 19, 2023 · 1 comment
Assignees
Labels
fix ready A fix was created. It's pending review or push.

Comments

@francois
Copy link

According to sl help revset:

"x::y"
 A DAG range, meaning all commits that are descendants of x and ancestors
 of y, including x and y themselves. If the first endpoint is left out,
 this is equivalent to "ancestors(y)", if the second is left out it is
 equivalent to "descendants(x)".

I wanted to rebase the two topmost commits back onto master. I expected the revset .^:: to be equivalent to "parent + self". Instead, I received a syntax error (I used log here, but I had the same error with rebase):

╰─○ sl log -r '.^::'
sl: parse error at 4: not a prefix: end
(.^::
     ^ here)

I tried to surround :: with spaces, but I had the same syntax error. The only way not to have a syntax error was to use the revset .^::..

@zzl0
Copy link
Contributor

zzl0 commented Apr 23, 2023

I wanted to rebase the two topmost commits back onto master

  • For your rebase use case, you can use sl rebase -s .^ -d master to achieve that.

sl: parse error at 4: not a prefix: end

For the parser error, seems a bug in the parser, it treats ^ as infix expr and :: as its right operand. I will dive into the issue next week.

@zzl0 zzl0 self-assigned this Apr 23, 2023
@zzl0 zzl0 changed the title Revset syntax error on ^.:: Revset syntax error on .^:: Apr 24, 2023
@zzl0 zzl0 added the fix ready A fix was created. It's pending review or push. label Apr 30, 2023
facebook-github-bot pushed a commit that referenced this issue May 1, 2023
Summary:
```
$ sl log -r '.^::'
sl: parse error at 4: not a prefix: end
(.^::
     ^ here)

```

Related issue: #612

Reviewed By: muirdm

Differential Revision: D45443281

fbshipit-source-id: c59e9bb5ebff08c9269e75c05f297675321c8557
facebook-github-bot pushed a commit that referenced this issue May 1, 2023
Summary:
The issue is that revset parser raised an error for `.^::`, the user's
expectation was something like `(.^)::`, but the current parser thinks `^` is
an infix in `.^::` [2], and then it tried to parse `::` as a prefix, and
eventually got the "not a prefix: end" error. I would agree with the user and
think `.^::` should be parsed as `(.^)::`.

I asked Yuya this question, the current parser can't recover from
the error while parsing. He suggested to backport a workaround from
https://www.mercurial-scm.org/repo/hg/rev/beb667c9880f.

This diff is to introduce that change to Sapling. Another potential fix might be
resolving this ambiguity in an ealier stage by checking token type after ^, so
we don't need the `_fixops` function to rewrite the tree, but that would need
more code.

Related issue: #612

Reviewed By: muirdm

Differential Revision: D45443282

fbshipit-source-id: 8eba6fbba76bdde5051342abc1d1c739eced2661
@zzl0 zzl0 closed this as completed May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix ready A fix was created. It's pending review or push.
Projects
None yet
Development

No branches or pull requests

2 participants