arklumpus / TreeViewer

Cross-platform software to draw phylogenetic trees
GNU Affero General Public License v3.0
183 stars 8 forks source link

Reading text in reverse from the command line #6

Closed TimothyStephens closed 3 years ago

TimothyStephens commented 3 years ago

Hi,

Thanks for creating such an awesome tool!

I have encountered a bug where TreeViewerCommandLine reads lines passed from the command line in reverse.

$ cat plot.txt
open test.tre
Y
$ TreeViewerCommandLine < plot.txt
TreeViewer v1.2.2 (command-line v1.1.1.0)

Type "help" for a list of available commands

Unknown command: ert.tset nepo
Unknown command: Y
>^D
>exit

Strangely enough redirecting the stdout fixes the problem.

$ TreeViewerCommandLine < plot.txt 1> plot.txt.log 
TreeViewer v1.2.2 (command-line v1.1.1.0)

Type "help" for a list of available commands

>open test.tre

  Suggested modules:

    Consensus (32914d41-b182-461e-b7c6-5f0263cc1ccd)
    Radial (95b61284-b870-48b9-b51c-3276f7d89df1)

  Would you like to load the suggested modules? [Y(es)/N(o)] Y

  Options for module Consensus (32914d41-b182-461e-b7c6-5f0263cc1ccd):

    No options available

  Options for module Radial (95b61284-b870-48b9-b51c-3276f7d89df1):

    ┌─────── Tree size ────────┐
    │ #1   Width:    2030      │
    │ #2   Height:    2030     │
    └──────────────────────────┘
    ┌─────── Tree area ────────┐
    │ #3   Start angle:    0   │
    │ #4   Sweep angle:    360 │
    └──────────────────────────┘

>
>^D
>exit

I have tested this on both Mac and Linux systems and the results are the same.

Cheers, Tim.

arklumpus commented 3 years ago

Hello!

Thank you for noticing this! It is a very interesting behaviour...

I think the issue is that, even when the input is being read from a file, TreeViewerCommandline still "pretends" to be reading from the console; when it tries to update the cursor position to the next character (line 571), for some reason it fails on macOS and Linux, but not on Windows (maybe because the first two don't allow us to move to an area where nothing has been written yet).

This causes the cursor position to be stuck at 0, and therefore each consecutive character gets inserted at the start of the command instead of the end.

Redirecting stdout solves the problem because it forces the code to take a different branch (line 372, line 582) where the commands are instead read line-by-line and then executed.

The solution would be to ensure that this second code path is also entered when stdin is redirected (and not only when stdout is redirected). This can be achieved by modifying line 372 from:

            if (!Console.IsOutputRedirected)

To

            if (!Console.IsOutputRedirected && !Console.IsInputRedirected)

Unfortunately, I am currently moving house and I will not have access to my computers at least for a week, which means that I will not be able to release a new version fixing the bug until then.

If this issue is a big annoyance for you, I would suggest redirecting stdout as a temporary remedy. You could also apply the fix above and recompile the program, but I'm not sure if it would be worth the effort 😅

TimothyStephens commented 3 years ago

Thanks for the quick response.

That is very interesting behaviour. Luckily, I normally redirect everything to a log file anyway. So no need to recompile, but thank you for the hotfix.

Cheers, Tim.