Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
Ian Lawrence Barwick barw...@gmail.com writes: 2013/9/10 Bruce Momjian br...@momjian.us: I still see that weird behavior in git head: pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. Should I revert the suggested patch? IIRC the patch was never applied, the reversion candidate is the existing commit 0725065b. I reverted 0725065b. AFAICT there is no interest in making \s produce a reliable full path name. There was some interest in making \cd tell you where it'd chdir'd to, but that would be a separate patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
On Tue, Jan 22, 2013 at 07:30:59PM -0500, Tom Lane wrote: Ian Lawrence Barwick barw...@gmail.com writes: Related email from the archives on this subject: http://www.postgresql.org/message-id/37ed240d0611200645l5b70c8ddw5fb735e0d35a7...@mail.gmail.com I agree with the opinion stated there that \cd with no argument really ought to do what cd with no argument usually does on the platform. So if we're going to fix \cd to print the resulting current directory, wouldn't it work to just set dir to . rather than / for Windows? Does commit 0725065b just need to be reverted, or is an additional patch required to remove the prefixed working directory from \s output? Offhand it looked like reverting the commit would be enough, but I didn't look hard to see if there had been any subsequent related changes. [ pokes around... ] Well, at least there are still no other uses of pset.dirname. I still see that weird behavior in git head: pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. Should I revert the suggested patch? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
2013/9/10 Bruce Momjian br...@momjian.us: On Tue, Jan 22, 2013 at 07:30:59PM -0500, Tom Lane wrote: Ian Lawrence Barwick barw...@gmail.com writes: Related email from the archives on this subject: http://www.postgresql.org/message-id/37ed240d0611200645l5b70c8ddw5fb735e0d35a7...@mail.gmail.com I agree with the opinion stated there that \cd with no argument really ought to do what cd with no argument usually does on the platform. So if we're going to fix \cd to print the resulting current directory, wouldn't it work to just set dir to . rather than / for Windows? Does commit 0725065b just need to be reverted, or is an additional patch required to remove the prefixed working directory from \s output? Offhand it looked like reverting the commit would be enough, but I didn't look hard to see if there had been any subsequent related changes. [ pokes around... ] Well, at least there are still no other uses of pset.dirname. I still see that weird behavior in git head: pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. Should I revert the suggested patch? IIRC the patch was never applied, the reversion candidate is the existing commit 0725065b. (Sorry for not following up earlier, this one dropped off my radar). Regards Ian Barwick -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
Ian Lawrence Barwick barw...@gmail.com writes: I've noticed a filename error in feedback messages from psql's '\s' command when saving the command line history to a file specified by an absolute filepath: psql (9.2.2) Type help for help. pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. The second and third '\s' commands display incorrect filepaths in the feedback message, despite writing correctly to the specified file. I wonder why we don't just revert commit 0725065b --- this complaint shows that it was quite poorly thought out, and I don't really see the need for it anyway. Aside from the complained-of bug, the code added to the \cd command is entirely incapable of tracking through multiple \cd operations properly. If we did think that this specific backslash command needed to be able to print something other than the filename as-entered, I'd be inclined to just apply make_absolute_path() to the name, instead of relying on inadequate dead-reckoning. However, that would require making make_absolute_path available in src/port/ or someplace, which seems a bit more than this feature is worth. Why should \s, and \s alone, need to remind you where you're cd'd to? Thoughts? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
2013/1/22 Tom Lane t...@sss.pgh.pa.us: Why should \s, and \s alone, need to remind you where you're cd'd to? Why not just get rid of that prefixed cd'd path in \s? []s -- Dickson S. Guedes mail/xmpp: gue...@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
I wrote: If we did think that this specific backslash command needed to be able to print something other than the filename as-entered, I'd be inclined to just apply make_absolute_path() to the name, instead of relying on inadequate dead-reckoning. However, that would require making make_absolute_path available in src/port/ or someplace, which seems a bit more than this feature is worth. Why should \s, and \s alone, need to remind you where you're cd'd to? It strikes me that a more useful reminder feature could be implemented by having \cd itself print the new current directory, which it could do with a simple call to getcwd(), thus not requiring refactoring of make_absolute_path. Then for instance if you'd forgotten where you were, \cd . would tell you. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
2013/1/23 Tom Lane t...@sss.pgh.pa.us: I wrote: If we did think that this specific backslash command needed to be able to print something other than the filename as-entered, I'd be inclined to just apply make_absolute_path() to the name, instead of relying on inadequate dead-reckoning. However, that would require making make_absolute_path available in src/port/ or someplace, which seems a bit more than this feature is worth. Why should \s, and \s alone, need to remind you where you're cd'd to? It strikes me that a more useful reminder feature could be implemented by having \cd itself print the new current directory, which it could do with a simple call to getcwd(), thus not requiring refactoring of make_absolute_path. Then for instance if you'd forgotten where you were, \cd . would tell you. \! pwd does the trick as well. However personally I prefer to get some kind of feedback from an action, so having \cd confirm the directory would be nice. I'll submit a patch. Related email from the archives on this subject: http://www.postgresql.org/message-id/37ed240d0611200645l5b70c8ddw5fb735e0d35a7...@mail.gmail.com Does commit 0725065b just need to be reverted, or is an additional patch required to remove the prefixed working directory from \s output? Ian Barwick -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
Ian Lawrence Barwick barw...@gmail.com writes: Related email from the archives on this subject: http://www.postgresql.org/message-id/37ed240d0611200645l5b70c8ddw5fb735e0d35a7...@mail.gmail.com I agree with the opinion stated there that \cd with no argument really ought to do what cd with no argument usually does on the platform. So if we're going to fix \cd to print the resulting current directory, wouldn't it work to just set dir to . rather than / for Windows? Does commit 0725065b just need to be reverted, or is an additional patch required to remove the prefixed working directory from \s output? Offhand it looked like reverting the commit would be enough, but I didn't look hard to see if there had been any subsequent related changes. [ pokes around... ] Well, at least there are still no other uses of pset.dirname. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] psql: small patch to correct filename formatting error in '\s FILE' output
I've noticed a filename error in feedback messages from psql's '\s' command when saving the command line history to a file specified by an absolute filepath: psql (9.2.2) Type help for help. pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. The second and third '\s' commands display incorrect filepaths in the feedback message, despite writing correctly to the specified file. Also, if the specified file could not be written to, the error message displayed formats the filepath differently (i.e. it does not prepend the current working directory), which is potentially confusing, and certainly visually inconsistent: pgdevel=# \cd /tmp pgdevel=# \s foo/history.txt could not save history to file foo/history.txt: No such file or directory pgdevel=# \! mkdir foo pgdevel=# \s foo/history.txt Wrote history to file /tmp/foo/history.txt. The attached patch rectifies these issues by adding a small function 'format_fname()' to psql/stringutils.c which formats the filepath appropriately, depending on whether an absolute filepath was supplied or psql's cwd is set. pgdevel_head=# \s history.txt Wrote history to file ./history.txt. pgdevel_head=# \s /tmp/history.txt Wrote history to file /tmp/history.txt. pgdevel_head=# \cd /tmp pgdevel_head=# \s /tmp/history.txt Wrote history to file /tmp/history.txt. pgdevel_head=# \cd /tmp pgdevel_head=# \s bar/history.txt could not save history to file /tmp/bar/history.txt: No such file or directory pgdevel_head=# \! mkdir bar pgdevel_head=# \s bar/history.txt Wrote history to file /tmp/bar/history.txt. Notes/caveats - The function 'format_fname()' deterimines whether the supplied filepath is absolute by checking for the presence of a '/' as the first character. This strikes me as a bit hacky but I can't think of an alternative. - As far as I can tell, Windows does not support the '\s' command, so there is presumably no need to worry about supporting Windows-style file paths - As far as I can tell, this is the only psql slash command which, after saving data to a file, provides a feedback message containing the filename/path. Regards Ian Lawrence Barwick psql-save-history-2013-01-21.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
