martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
9.07k stars 317 forks source link

`jj untrack foo~` fails #4509

Open 0-wiz-0 opened 1 month ago

0-wiz-0 commented 1 month ago

Description

emacs writes backup files with a ~ (tilde) at the end. jj automatically picks them up if you forgot to ignore them in the .gitignore file. I can't untrack them in the latest version any longer. (I think this worked when it was jj untrack, but now it doesn't with jj file untrack, but perhaps I misremember.)

Steps to Reproduce the Problem

  1. jj git init
  2. touch foo~
  3. jj
  4. jj file untrack foo~
    jj file untrack foo\~
    Error: Failed to parse fileset: Syntax error
    Caused by:  --> 1:5
    |
    1 | foo~
    |     ^---
    |
    = expected `~` or <primary>

Specifications

yuja commented 1 month ago

Since fileset is enabled by default, ~ has to be quoted: '"foo~"' or 'glob:"*~"'. https://martinvonz.github.io/jj/latest/filesets/

Maybe we can add a hint if the expression looks like an existing path.

0-wiz-0 commented 1 month ago

From a quick read of the linked page: ~ seems only to be supported as a separate word, or at the start of a word, so perhaps making that an explicit requirement would also solve this.

yuja commented 1 month ago

Yeah, trailing ~ could be special cased, but I don't think it's worth the added oddity. *~ is usually a backup file and so often ignored by .gitignore or user ignore.

x~y and ~x are valid syntax, btw.

emilazy commented 1 month ago

You can add this stuff to your global Git ignores in ~/.config, by the way.

samueltardieu commented 1 month ago

The message is now:

Error: Failed to parse fileset: Syntax error
Caused by:  --> 1:5
  |
1 | foo~
  |     ^---
  |
  = expected `~` or <primary>
Hint: See https://martinvonz.github.io/jj/latest/filesets/ for filesets syntax, or for how to match file paths.

Hopefully this is a clearer message. If this is, might we close this issue?

0-wiz-0 commented 1 month ago

I accept that I'm the only one annoyed by the change in behaviour between the old 'jj untrack' and the new 'jj file untrack' and have worked around it by having my global git config ignore "*~" files.

As for the actual change - I think this needs improvements in the documentation. I read expected~or <primary> as that jj would expect a second tilde or a <primary> after the first tilde. Both alternatives are unclear to me. Neither https://martinvonz.github.io/jj/latest/filesets/ nor the page it links to from 'must be quoted', https://martinvonz.github.io/jj/latest/templates/#string-literals , mention ~~, and in my reading of https://martinvonz.github.io/jj/latest/templates/#string-literals I don't see a mention of how I should quote ~ at all, since the full list of backslash-quoted characters doesn't include the tilde The word primary is not mentioned in the filesets page at all, so I don't know what a <primary> is either.

martinvonz commented 1 month ago

the change in behaviour between the old 'jj untrack' and the new 'jj file untrack'

FYI, I don't think it's a difference between jj untrack and jj file untrack. jj untrack actually still exists as an alias. What changed is that we enabled filesets by default.

I read expected~or <primary> as that jj would expect a second tilde or a <primary> after the first tilde.

~ is simply an operator for subtraction/difference (that's why it looks similar to -, which we chose not to use because it's so common in filenames).

A second tilde followed by an expression would make the syntax correct, but it's probably not what. The error message is generated by the low-level parsing code without understanding what makes a useful expression. foo~~bar means "file foo but not (not file bar)`, i.e. an empty set.

I think you can think of a <primary> is a fileset expression.

Neither https://martinvonz.github.io/jj/latest/filesets/ nor the page it links to from 'must be quoted',

That page includes this:

File names passed to these commands must be quoted if they contain whitespace or meta characters.

In foo~, the ~ is such a "meta character". @yuja, should we simply call it "operator" for simplicity? I'm guessing "meta characters" also includes the : in glob: etc., but do we need make that distinction? If we do, we should probably document it.

As for the actual change - I think this needs improvements in the documentation.

I'm not arguing with this. The above is just to explain how it actually works.

ilyagr commented 1 month ago

Maybe we can add a hint if the expression looks like an existing path.

This sounds like a good idea to me, it's consistent with what we try to do in other similar cases. I just wanted to resurface it so that it doesn't get lost in the discussion.

ilyagr commented 1 month ago

One (not fully thought through) possibility for improving the docs would be to change the URL in the error Samuel quoted in https://github.com/martinvonz/jj/issues/4509#issuecomment-2395530455 to link to a section, e.g. https://martinvonz.github.io/jj/latest/filesets/#syntax-error. Then, we could put a few examples into that section (mainly for unexpected syntax errors).

I was originally going to suggest combining this with Yuya's idea I quoted in my previous message, but while doing both makes sense to me, maybe having the URL would work even without implementing an error specific to patterns that look like files.

yuja commented 1 month ago

In foo~, the ~ is such a "meta character". @yuja, should we simply call it "operator" for simplicity? I'm guessing "meta characters" also includes the : in glob: etc., but do we need make that distinction?

I just thought "meta character" is more abstract and easier to imagine than "operator". If we need a strict definition, maybe we can add a link to the grammar file?

Maybe we can add a hint if the expression looks like an existing path.

This sounds like a good idea to me, it's consistent with what we try to do in other similar cases.

I thought this would be a nice addition, but I think the current state (a hint with doc URL) is pretty good. We can improve the doc by adding more bad/good examples. (If I were new to jj, I would probably search the doc by error message.)

taranlu-houzz commented 4 days ago

Hello, I am running into a similar issue with a file that got accidentally tracked that has a single quote: My Test's test.xlsx and I cannot seem to get jj to untrack the file. I've tried different variations of quoting and escaping characters, but nothing has worked for me and I always get this error:

❯ jj file untrack "My Test's test.xlsx"
Error: Failed to parse fileset: Syntax error
Caused by:  --> 1:8
  |
1 | My Test's test.xlsx
  |        ^---
  |
  = expected <EOI>
martinvonz commented 4 days ago

The quoting depends on your shell. Which shell do you use? I think in Bash, this would work:jj file untrack '"My Test'\''s test.xlsx"'

taranlu-houzz commented 4 days ago

@martinvonz I was just about to update my comment with that. I use fish, and that is exactly what ended up working.

martinvonz commented 4 days ago

Btw, if you don't want to have to remember the escaping rules, you could instead do jj untrack 'glob:My Test*.xslx' (or whatever glob is sufficient to make it match only the desired file).

scott2000 commented 1 day ago

Maybe it would be a good idea to add a global --raw-paths option which disables fileset parsing for command line arguments. Then the error message could give a simple solution without having to explain quoting, like "This argument is being interpreted as a fileset. If you want to use a plain file path, consider using `--raw-paths` to prevent it from being interpreted as a fileset."

This would probably also be good for shell scripts to avoid having to escape paths. It's a bit different from the current ui.allow-filesets option, since it would only apply to command line arguments, not filesets in other places.

ilyagr commented 1 day ago

For reference, there was a related discussion on Discord (+ thread ) where somebody encountered this error:

$ jj restore emacs/.emacs.d/lisp/\#init-os.el\# 
Error: Failed to parse fileset: Syntax error
Caused by:  --> 1:21
  |
1 | emacs/.emacs.d/lisp/#init-os.el#
  |                     ^---
  |
  = expected <EOI>, `|`, `&`, or `~`
Hint: See https://martinvonz.github.io/jj/latest/filesets/ for filesets syntax, or for how to match file paths.

Adding to the confusion, # is not currently a special fileset symbol.

@bnjmnt4n found a related TODO:

https://github.com/martinvonz/jj/blob/528ccb318e084d10d4951ef976b3c0b1729e0413/lib/src/fileset.pest#L30-L36

yuja commented 1 day ago

Maybe it would be a good idea to add a global --raw-paths option which disables fileset parsing for command line arguments.

If we need an escape hatch, a modifier syntax might be better. Something like raw:<PATH> or literal:<PATH>.

For this emacs example, we can add # to the base string characters.