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

[Benno Schulenberg]

On Sat, Jan 9, 2016, at 20:43, Rishabh Dave wrote:
> If any one does './nano -G nonexistentdir/folder' and edits the buffer 2
> things must happen: (a) launch new buffer properly

Yes.

> and (b) skip 'lockfile business'

Yes, skip, not switch off.

> 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.

No, that is even worse than a single global variable: now you
have this variable for /every/ open file.  But it is needed only
during the brief moment when trying to find and open a file;
after that, it should be gone.

> Patch works nicely here, I suppose. But when do this './nano folder' it
> opens up new buffer but with "New File" message.

Not good...

Also, if I do 'src/nano doc/doc/doc', it says that
directory '/doc' doesn't exist.  But I didn't mention
any /doc, I specified the nonexistent dir 'doc/doc'.

> 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.

Me too.  That is part of the reason why this bug isn't fixed yet.  :)

But... see attached patch for an improvement.
(It is against SVN; things changed a bit since 2.5.0.)
It seems to work for me.

> Also, if all that is valid and I am incorrect how am I
> supposed to save message from overlapping each other?

By not printing the message when you discover the fault,
maybe, but only when otherwise "New File" would be printed?
Or... maybe you can leave things as they are, but just skip
printing "New File" when faulty_path is TRUE?


Some details about the patch:

     if (fd < 0 || filestream == NULL) {
        statusbar(_("Error writing lock file %s: %s"), lockfilename,
-                   strerror(errno));
+                   strerror(errno));
        goto free_and_fail;
     }

You added trailing whitespace there.  Please don't do that.

+        ssize_t readtot = 0;
+        ssize_t readamt = 0;
+       char *lockbuf = charalloc(8192);
+       char *promptstr = charalloc(128);

Also here, and in the surrounding stuf, you modified the
whitespace.  Don't do that.  Restore it as it was.

+    if ((fault = fault_in_path(filename)) != NULL) {
+        statusbar(_("New File. Directory \'%s\' doesn't exist"), fault);

Don't say "New File", just say that the dir does not exist.
Error messages should override any informational stuff.

+        UNSET(LOCKING);

No, it should *not* unset LOCKING.  There may be multple
files on the command line; for any that exist, locking should
be executed when LOCKING is set.  So when a specified dir
does not exist, the locking business should just be *skipped*,
not switched off.

Also, instead of 'do {} while (TRUE)', just
use 'while (TRUE) {}'.

     ans = mallocstrcpy(NULL,
 #ifndef NANO_TINY
-       (!exiting && openfile->mark_set) ? "" :
+       ((!exiting && openfile->mark_set) || openfile->faulty_path) ? "" :

No, it shouldn't set the path to blank when the path is faulty;
keep the faulty path; the user just needs to be warned that it
doesn't exist; whatever she specified shouldn't be discarded.

-    if (openfile->filename[0] == '\0') {
+    if ((openfile->filename[0] == '\0' || openfile->faulty_path )) {
        prefix = _("New Buffer");

Same thing; don't say it's a new buffer when you have a path,
even when the path is invalid.


> Apart from this where are function definition of _() and beep()?

For _() see in src/nano.h:
#define _(string) gettext(string)

So _() is a shorthand for gettext(); it gives back a translation
of the passed string.

For beep() see 'man 3n beep'; it is part of ncurses.

Benno

-- 
http://www.fastmail.com - Email service worth paying for. Try it for free

warn-about-nonexistent-directory.patch
Description: Text Data