taskwarrior icon indicating copy to clipboard operation
taskwarrior copied to clipboard

[TW-1821] Escaping of '/'s in modify/substitution command confuses the parser

Open taskwarrior opened this issue 7 years ago • 3 comments

Johannes Schlatow on 2016-06-23T12:11:18Z says:

This is related to TW-295. Although the test case is working properly, I noticed that the parses does not recognise the substitution in other (corner) cases.

The test case works by substituting '/two/' with "TWO" by using the following command:

task 1 modify /\\/two\\//TWO/

However, if I only want to match for '/two', the parser does not recognise the /match/replace/ pattern, i.e.:

task 1 modify /\\/two/TWO/

replaces the entire description with '//two/TWO/'.

taskwarrior avatar Feb 14 '18 19:02 taskwarrior

Migrated metadata:

Created: 2016-06-23T12:11:18Z
Modified: 2016-06-23T19:38:37Z

taskwarrior avatar Feb 14 '18 19:02 taskwarrior

The reason for this is, that the differentiation between path and subsitution is based on the slash count, see Lexer.cpp:903

bool Lexer::isPath (std::string& token, Lexer::Type& type)
{
  std::size_t marker = _cursor;
  int slashCount = 0;

  while (true)
  {
    if (_text[marker] == '/')
    {
      ++marker;
      ++slashCount;
    }
    else
      break;

    if (_text[marker] &&
        ! unicodeWhitespace (_text[marker]) &&
        _text[marker] != '/')
    {
      utf8_next_char (_text, marker);
      while (_text[marker] &&
             ! unicodeWhitespace (_text[marker]) &&
             _text[marker] != '/')
        utf8_next_char (_text, marker);
    }
    else
      break;
  }

  if (marker > _cursor &&
      slashCount > 3)
  {
    type = Lexer::Type::path;
    token = _text.substr (_cursor, marker - _cursor);
    _cursor = marker;
    return true;
  }

  return false;
}

therefore, the second substitution is wrongly detected as a path, whereas the first one is working, since due to the double slash, the parser recognizes that this cannot be a path.

task 1 modify /\\/two//

would work as well (tested). I think we also have to ignore escaped slashes in the path.

sebu06 avatar Aug 28 '21 08:08 sebu06

@sebu06 I think you might be on the right track here. Perhaps let's try to convert this to a PR? Then we can add more tests to ensure this use case is covered :rocket:

tbabej avatar Sep 04 '21 16:09 tbabej