GNU bug report logs - #23566
25.0.94; sql-sqlite: selecting database file is crippled

Previous Next

Full log

View this message in rfc822 format

From: Rolf Ade <rolf <at> pointsman.de>
To: npostavs <at> users.sourceforge.net
Cc: 23566 <at> debbugs.gnu.org
Subject: bug#23566: sql-sqlite: selecting database file is crippled)
Date: Tue, 03 Jan 2017 01:19:46 +0100

Thank you to take care.

npostavs <at> users.sourceforge.net writes:
> Rolf Ade <rolf <at> pointsman.de> writes:
>
>> Therefor I propose this slightly more elaborate patch:
>>
>> [...]
>>    :version "24.1"
>
> The :version should be updated when the default value is changed.

Done, see below.
>
>> [...]
>> +                             (if (not(file-regular-p f))
>                                        ^^
>                                        missed a space here

Thanks for the review and for spotting this.

> This looks reasonable to me, and is small enough to install without
> copyright assignment.  Could you add a commit message too? (format as
> described in CONTRIBUTE)

Commit message:

Fix selecting SQLite database files with sql-mode (bug#23566)

* lisp/progmodes/sql.el (sql-sqlite-login-params): Allow any name as
SQLite database file name, by default.
(sql-get-login-ext): Fixed read-file-name arguments to provide
path completion even if a database name pattern is customized and to
allow creation of new SQLite database files.

Copyright-paperwork-exempt: yes

>> There is no need for fear, a user may by mistake damage a non SQLite
>> file (by not restricting the file select to a small number of file name
>> suffixes (which would't be a great strategy. if such fear had ground).
>> Every SQLite database file has a 100 byte long database header described
>> here: https://www.sqlite.org/fileformat.html#the_database_header
>
> I think the idea is more to restrict the completion to database files
> for convenience, not protection from damage.  But since there doesn't
> seem to be much of a convention for sqlite file extensions, setting the
> default to nil seems okay.

I agree. Most probably, the idea was to provide some
convenience. Therefor, I didn't removed the whole machinery but fixed
it.

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index d6c9516..55ba320 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -933,10 +933,10 @@ sql-sqlite-options
   :version "20.8"
   :group 'SQL)
 
-(defcustom sql-sqlite-login-params '((database :file ".*\\.\\(db\\|sqlite[23]?\\)"))
+(defcustom sql-sqlite-login-params '((database :file nil))
   "List of login parameters needed to connect to SQLite."
   :type 'sql-login-params
-  :version "24.1"
+  :version "26.1"
   :group 'SQL)
 
 ;; Customization for MySQL
@@ -2958,9 +2958,11 @@ sql-get-login-ext
                         (file-name-nondirectory last-value)
                         (when (plist-get plist :file)
                           `(lambda (f)
-                             (string-match
-                              (concat "\\<" ,(plist-get plist :file) "\\>")
-                              (file-name-nondirectory f)))))))
+                             (if (not (file-regular-p f))
+                                 t
+                               (string-match
+                                (concat "\\<" ,(plist-get plist :file) "\\>")
+                                (file-name-nondirectory f))))))))
 
       ((plist-member plist :completion)
        (completing-read prompt-def (plist-get plist :completion) nil t

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.