Re: [Nano-devel] Patch for bug #44950

[Benno Schulenberg]

Hi Rishabh,

On Tue, Jan 5, 2016, at 20:37, Rishabh Dave wrote:
> I thought if I am solving bug for nano, that could only be done using some
> other text editor! :P

No no, the true bug solver uses the tool containing the bug
to solve the bug.  :)

> > And finally, when running './nano foo/bar/baz', with your patch
> > nano would say that dir 'foo/bar' does not exist, whereas I would
> > expect it to say that dir 'foo' does not exist.  I admit, it is
> > harder to make, but the message would be nicer, more natural.
> 
> I achieved it

(Sorry, I didn't read your whole email before starting to answer;
I see your second patch tries to solve the --locking alternative.
But still, it does not in the desired way I describe below.)

Well, yes, you it achieved for the case without --locking.  But when
--locking is used, the statusbar still says "Error writing lock file: ...".
That is not what I want.  It should say nothing about lockfiles, it
shouldn't even try to write a lockfile in a place that doesn't exist,
it should avoid and skip the whole lockfile business when the user
specifies a nonexistent directory in the filename path.

> but I think code might be little ugly,

It is, somewhat.  For example, instead of doing

  if (something) {
      one;
      two;
      ....
  } else
      return NULL

I would do:

  if (!something)
      return NULL

  one;
  two;
  ....

Much clearer.

> so I added comments
> over every if-statements explaining them quickly.

That's fine.

> After changing function's return type to bool and after covering case of
> locking files, it struck me it will be more appropriate to name it
> 'fault_in_path()'.

Good.  But don't worry too much about the naming,
I will likely pick another one when the patch goes in.

> It would also make code readable, for example - b =
> fault_in_path(a/b/c/d). Feeling this was appropriate I changed name of our
> variable 'mistakes_in_path' to 'faulty_path'. Even that is more readable
> now, I suppose (e.g. faulty_path = TRUE).

But why do you need this global variable anyway?
It would be nicer if you could make it so that you
don't need it.

> The major change apart from making message specific, was changing return
> type of fault_in_path() - which was determine() before. Code responsible
> for file locking already checks if path exists and displays incorrect path

Yes, and that is what I want to get rid off -- not the locking code should
report a nonexistent dir, the code /before it/ should, and if it does so, it
should silently skip trying to write a lock file.

> Now, these ones are only trivial. It is not possible to detect multiple
> errors in path, only outermost incorrect directory can be detected. There
> is no economic way of even guessing multiple incorrect directories, right?

No, there is not.

> Other thing, is it good to re-use variables in same function for same
> purpose but over different objects?

That is fine.  But don't name them len1 and len2, that's ugly;
name them lenthis and lenparent, or something else that helps to
make the code more understandable.

> And... Am I asking too many questions?

Ehm...  With this last one, yes.  :)

Benno

-- 
http://www.fastmail.com - Access all of your messages and folders
                          wherever you are