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

[Rishabh Dave]

O oh, now I feel I am coming towards the problem you kept speaking about. Sir, I am sorry about for a delay.

If any one does './nano -G nonexistentdir/folder' and edits the buffer 2 things must happen: (a) launch new buffer properly and (b) skip 'lockfile business' i.e. unset locking for new file (because write_lockfile function then complains about lock file doesn't exist and also because you said. :P ).

Variable, faulty_path is no more global variable. But we need to denote "New Buffer" which could only be done through tittlebar() in winio.c. Other alternatives I thought of were declaring it as enum, running fault_in_path() in winio.c, too. Then I came across, bool variable 'modified' in structure openfilestruct declared in nano.h. Thinking that declaring faulty_path here would be more appropriate, I made it as part of openfilestruct.

Patch works nicely here, I suppose. But when do this './nano folder' it opens up new buffer but with "New File" message. I investigated for hours and also did it multiple times, so finally I decided to report to you.

Function call to fault_in_path() is now only in open_buffer(). There is something awkward around it. Previous block of code is supposed to check if file is not new, if it is directory and then returns control. I have confirmed over many times - that it doesn't return control, instead it prints " is a directory" message and continues to next block where "New File" is printed over previous message. I failed to comprehend flow of control there. Also, if all that is valid and I am incorrect how am I supposed to save message from overlapping each other?

Apart from this where are function definition of _() and beep()? Neither grep search over source code nor a google search over internet helped.

Thank you for your consideration. Thank you.

On Thu, Jan 7, 2016 at 12:39 AM, Benno Schulenberg <address@hidden> wrote:

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

bug#44950.patch
Description: Text Data