[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Nano-devel] Patch for bug #44950
From: |
Benno Schulenberg |
Subject: |
Re: [Nano-devel] Patch for bug #44950 |
Date: |
Wed, 06 Jan 2016 20:09:04 +0100 |
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
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/03
- Re: [Nano-devel] Patch for bug #44950, Benno Schulenberg, 2016/01/03
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/05
- Re: [Nano-devel] Patch for bug #44950,
Benno Schulenberg <=
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/09
- Re: [Nano-devel] Patch for bug #44950, Mike Frysinger, 2016/01/09
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/10
- Re: [Nano-devel] Patch for bug #44950, Benno Schulenberg, 2016/01/11
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/11
- Re: [Nano-devel] Patch for bug #44950, Benno Schulenberg, 2016/01/12
- Re: [Nano-devel] Patch for bug #44950, Rishabh Dave, 2016/01/15
- Re: [Nano-devel] Patch for bug #44950, Benno Schulenberg, 2016/01/15
- Re: [Nano-devel] Patch for bug #44950, Benno Schulenberg, 2016/01/15
- Re: [Nano-devel] Patch for bug #44950, Mike Frysinger, 2016/01/15