Issue789

Title File name provided with "--plan-file" transformed to lowercase
Priority bug Status resolved
Superseder Nosy List jendrik, malte, mkatz, patfer
Assigned To patfer Keywords
Optional summary
From Fast Downward mailing list:

On 14.05.2018 18:30, Michael Katz wrote:
> Hi,
> 
> The planner seems to be changing the file name provided with
> /--plan-file /​to a lowercase. Was it done on purpose? Is that a bug? Is
> that a feature?

Hi Michael,

I would say that's a bug. The command-line parser in general is
case-insensitive, and this is implemented by transforming everything to
lower-case very early on during the processing. But this should not have
been done for the plan file name. If you open an issue for this, we will
hopefully look into it soonish. (It should be in category "bug".)

> I cannot seem to find where it was actually done.

It is probably the line commented with "// Ignore case." in
OptionParser::parse_cmd_line, file src/search/options/option_parser.cc.

Looking at that code, the next step looks dubious, too. Newlines should
be replaced with spaces, not with nothing, because stuff like "lm
count" should not be a legal way to write "lmcount".

Best,
Malte

Created on 2018-05-14.20:38:31 by mkatz, last changed by jendrik.

Summary
From Fast Downward mailing list:

On 14.05.2018 18:30, Michael Katz wrote:
> Hi,
> 
> The planner seems to be changing the file name provided with
> /--plan-file /​to a lowercase. Was it done on purpose? Is that a bug? Is
> that a feature?

Hi Michael,

I would say that's a bug. The command-line parser in general is
case-insensitive, and this is implemented by transforming everything to
lower-case very early on during the processing. But this should not have
been done for the plan file name. If you open an issue for this, we will
hopefully look into it soonish. (It should be in category "bug".)

> I cannot seem to find where it was actually done.

It is probably the line commented with "// Ignore case." in
OptionParser::parse_cmd_line, file src/search/options/option_parser.cc.

Looking at that code, the next step looks dubious, too. Newlines should
be replaced with spaces, not with nothing, because stuff like "lm
count" should not be a legal way to write "lmcount".

Best,
Malte
Messages
msg7409 (view) Author: jendrik Date: 2018-09-12.16:12:12
This has already been merged a while ago.
msg7406 (view) Author: malte Date: 2018-09-12.15:54:57
Hi all, I think the code for this was mostly done, so just a small question what
the status is here, and in particular if this is waiting for me to do something. :-)
msg7347 (view) Author: jendrik Date: 2018-08-19.18:30:31
I ran into this issue myself today. Patrick, could you please make the tests and 
the modification (comment on Bitbucket) that Malte suggested in msg7304 and merge 
this?
msg7304 (view) Author: malte Date: 2018-07-18.19:31:53
I left one more small comment about adding a comment. Did you test that this now
works correctly with filenames containing uppercase letters or newlines? No
further comments from my side, looks good to merge if it works.
msg7303 (view) Author: patfer Date: 2018-07-14.14:28:33
Comments by Malte incorporated.
msg7302 (view) Author: malte Date: 2018-07-12.16:04:43
I left two additional comments.
msg7298 (view) Author: jendrik Date: 2018-07-11.08:23:27
Patrick has made the suggested changes and the code looks good to me now.
msg7292 (view) Author: jendrik Date: 2018-07-10.07:32:22
I'm done with my review and left a few comments on Bitbucket.
msg7291 (view) Author: jendrik Date: 2018-07-10.07:07:11
I'll have a look.
msg7290 (view) Author: patfer Date: 2018-07-10.05:15:58
Implemented desired fixes:
1. '\n' in arguments replaced by ' '
2. --plan-file is now parsed case sensitive

For Pull Request:
https://bitbucket.org/PatFer/downward/pull-requests/3/pr-issue789/diff

Any review volunteering?
msg7289 (view) Author: malte Date: 2018-07-09.15:15:59
It would be good to fix this, but I don't currently have cycles to spare.
Michael, if you want to see this fixed soon, I think your best chance is to
lobby one of the people in the Basel group to see if they might want to work on
this.
History
Date User Action Args
2018-09-12 16:12:12jendriksetstatus: reviewing -> resolved
messages: + msg7409
2018-09-12 15:54:57maltesetmessages: + msg7406
2018-08-19 18:30:31jendriksetmessages: + msg7347
2018-07-18 19:31:53maltesetmessages: + msg7304
2018-07-14 14:28:33patfersetmessages: + msg7303
2018-07-12 16:04:43maltesetmessages: + msg7302
2018-07-11 08:23:27jendriksetmessages: + msg7298
2018-07-10 07:32:22jendriksetmessages: + msg7292
2018-07-10 07:07:11jendriksetnosy: + jendrik
messages: + msg7291
2018-07-10 05:15:58patfersetstatus: chatting -> reviewing
messages: + msg7290
2018-07-10 03:20:42patfersetassignedto: patfer
nosy: + patfer
2018-07-09 15:15:59maltesetstatus: unread -> chatting
messages: + msg7289
2018-05-14 21:43:05maltesetnosy: + malte
2018-05-14 20:48:51mkatzsettitle: ile name provided with "--plan-file" transformed to lowercase -> File name provided with "--plan-file" transformed to lowercase
2018-05-14 20:38:31mkatzcreate