← Back to team overview

cuneiform team mailing list archive

Re: Checked in basic User Dictionary support

 

On Mon, Jul 13, 2009 at 1:30 AM, Ben Jackson<ben@xxxxxxx> wrote:

>> Just a quick note to you all: don't run these kinds of commands as
>> root. First you compile the dict and only then do a 'sudo cp ...'.
>
> That's an excellent idea, but it won't work.  You'll have to do it the
> way I described until someone modifies the path handling to support
> local files.  As it stands it will always prepend the data dir to
> whatever you supply, even if it starts with a leading /.

I went through the code at last. Here are a few issues I found.

Firstly the indenting. You seem to be using a mixture of tabs and
spaces. All new code should use indenting of 4 spaces without any
tabs. (Yes, the indentation style changes wildly between files. Yes,
we should fix that.)

As far as the data file opening goes, I would like to get some
comments from Cognitive. What is the point of TGOPEN, TGREAD etc
functions? They are mapped to TG_open etc files that seem to hold a
cache of 16 opened files. Why is this? What is the problem it tries to
solve?

If it were up to me, I'd replace the TG* calls in CloseUserDictionary
with regular fopen/etc calls. Especially since that function is not
called anywhere else in the entire source tree. Does Cognitive use
this internally? Do we need to preserve its behavior?

Cuneiform-dict uses getopt to parse command line arguments. This is
unavailable in MSVC, and we need to support that too. This is one of
the reasons why the cli executable parses arguments manually.

You should use the same copyright header as the other files (the one
with the gibberish russian copy of the BSD license).



References