tuura / scenco-core

Collection of encoding algorithms for conditional graphs
Other
0 stars 1 forks source link

Check for Abc and for the gate library #28

Closed allegroCoder closed 8 years ago

allegroCoder commented 8 years ago
snowleopard commented 8 years ago

ABC is not installed properly in your machine Gate library not defined properly

Can we avoid words like "properly" etc.? They don't carry any useful meaning for the user. Be more specific. What exactly would you like to say? That you didn't find ABC. Then tell this: "ABC not found". If you want to provide more useful message, use "ABC synthesis tool not found".

Same with the library: "Gate library not found". And then maybe tell which one, to make the message more helpful.

Final comment: you need to get rid of code duplication when checking ABC.

snowleopard commented 8 years ago

And, by the way, this check is clearly wrong:

abcCommand :: FilePath
abcCommand = "abc"

abcCheck :: IO Bool
abcCheck = doesFileExist abcCommand

If ABC is installed somewhere in /long/path/to/abc your doesFileExist "abc" will fail.

allegroCoder commented 8 years ago

I did what you asked. Just a question about your last comment: do you mean that I should check the existance of ABC by running something on it? A simple command to figure out if it is ABC and not just a text file (for instance) named "abc"?

@EDIT

In the README there are the instructions on how to set ABC path into the system path, therefore I assume that ABC path is not introduced as a command line option by giving the path, but it is already set in the system path. Therefore, if ABC is in /long/path/ I assumed that the /long/path is set inside within the system path. Do you want me to add an option for manually set ABC path? Is this what you mean?

I would display the following message when ABC is not found:

ABC synthesis tool not found.
Read README for the instructions on how to install and set ABC in the system path.
snowleopard commented 8 years ago

@allegroCoder Look at findExecutable in System.Directory package. An example of using it can also be found here: https://github.com/snowleopard/hadrian/blob/master/src/Oracles/LookupInPath.hs#L20-L25. (You don't need liftIO or unifyPath -- your case is simpler.)

snowleopard commented 8 years ago

The above changes look good apart from some redundant brackets when printing error. Let me know once you are happy and I can merge this.

allegroCoder commented 8 years ago

I removed some of the brackets. Let me know once you merge this PR @snowleopard , so that I will make the other PR for the other branch

snowleopard commented 8 years ago

Thanks! Now squashed by GitHub and merged.