bug#25025: python-shell-calculate-command is wrong

bug#25025: python-shell-calculate-command is wrong

[Fabrice Popineau]

[-- Attachment #1: Type: text/plain, Size: 24181 bytes --]

Run Emacs-W64 from an MSYS2 shell (bash)

In your init file:

(setq python-shell-interpreter
      "c:/Local/Miniconda3/Scripts/ipython.exe")

and then launch a python shell.
It results in this weird error that
No such file or directory
"c\:/Local/Miniconda3/Scripts/ipython.exe"

Culprit is `python-shell-calculate-command' which wrongly
uses `shell-quote-argument' on python-shell-interpreter.

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          (shell-quote-argument python-shell-interpreter)
          python-shell-interpreter-args))

IMHO, it should read:

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          python-shell-interpreter
          python-shell-interpreter-args))

at least for windows-nt. The python shell name is not passed to any
underlying shell.
It is used to create a process, so it must not be quoted in anyway.

Regards,

Fabrice


In GNU Emacs 25.1.50.25 (x86_64-w64-mingw32)
 of 2016-11-24 built on LOBSANG
Repository revision: 43f4fc4a57af39b4a7f166f27dcf07fc9e8e2e3b
Windowing system distributor 'Microsoft Corp.', version 6.2.9200
Configured using:
 'configure --prefix=/c/Local/Emacs-25
 --libexecdir=/c/Local/Emacs-25/bin --datarootdir=/c/Local/Emacs-25
 --localstatedir=/c/Local/Emacs-25 --sysconfdir=/c/Local/Emacs-25/etc
 --with-jpeg --with-xpm --with-png --with-tiff --with-rsvg --with-xml2
 --with-gnutls --with-imagemagick --without-dbus --enable-checking=no
 'CFLAGS=-I/mingw64/include -fomit-frame-pointer -O3 -g0 -mtune=corei7'
 CPPFLAGS=-I/mingw64/include LDFLAGS=-L/mingw64/lib'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND NOTIFY ACL GNUTLS LIBXML2
ZLIB TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: en_US
  locale-coding-system: cp1252

Major mode: Emacs-Lisp

Minor modes in effect:
  popwin-mode: t
  pdf-occur-global-minor-mode: t
  pyvenv-mode: t
  company-statistics-mode: t
  rainbow-delimiters-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-anzu-mode: t
  anzu-mode: t
  projectile-mode: t
  desktop-save-mode: t
  TeX-PDF-mode: t
  winner-mode: t
  which-key-mode: t
  which-function-mode: t
  volatile-highlights-mode: t
  save-place-mode: t
  savehist-mode: t
  recentf-mode: t
  global-page-break-lines-mode: t
  page-break-lines-mode: t
  outline-minor-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  flx-ido-mode: t
  ido-vertical-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  global-hungry-delete-mode: t
  hungry-delete-mode: t
  global-hl-line-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  diff-auto-refine-mode: t
  beacon-mode: t
  global-auto-revert-mode: t
  cua-mode: t
  delete-selection-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  prettify-symbols-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  global-visual-line-mode: t
  visual-line-mode: t
  transient-mark-mode: t

Recent messages:
Mark set
Quit
Mark set
next-line: End of buffer
Quit
uncompressing python.el.gz...done
You can run the command ‘find-function’ with C-h C-f
uncompressing python.el.gz...done
Mark set
Mark saved where search started

Load-path shadows:
c:/Home/.emacs.d/local/org-mode/lisp/htmlize hides
c:/Home/.emacs.d/elpa/htmlize-20130207.1202/htmlize
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-virtual hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-virtual
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-view hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-view
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-util hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-util
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-tools
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools-pkg hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-tools-pkg
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools-autoloads hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-tools-autoloads
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-sync hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-sync
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-outline hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-outline
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-occur hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-occur
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-misc hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-misc
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-links hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-links
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-isearch hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-isearch
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-info hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-info
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-history hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-history
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-dev hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-dev
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-cache hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-cache
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-annot hides
c:/Home/.emacs.d/elpa/pdf-tools-20161026.1557/pdf-annot
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-virtual hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-virtual
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-view hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-view
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-util hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-util
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-tools
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools-pkg hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-tools-pkg
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-tools-autoloads hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-tools-autoloads
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-sync hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-sync
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-outline hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-outline
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-occur hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-occur
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-misc hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-misc
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-links hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-links
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-isearch hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-isearch
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-info hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-info
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-history hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-history
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-dev hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-dev
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-cache hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-cache
c:/Home/.emacs.d/local/pdf-tools-0.70/pdf-annot hides
c:/Local/Emacs/site-lisp/pdf-tools-0.70/pdf-annot
c:/Home/.emacs.d/elpa/tablist-20160424.235/tablist hides
c:/Local/Emacs/site-lisp/tablist/tablist
c:/Home/.emacs.d/elpa/tablist-20160424.235/tablist-pkg hides
c:/Local/Emacs/site-lisp/tablist/tablist-pkg
c:/Home/.emacs.d/elpa/tablist-20160424.235/tablist-filter hides
c:/Local/Emacs/site-lisp/tablist/tablist-filter
c:/Home/.emacs.d/elpa/tablist-20160424.235/tablist-autoloads hides
c:/Local/Emacs/site-lisp/tablist/tablist-autoloads
c:/Home/.emacs.d/local/org-mode/lisp/ox hides c:/Local/Emacs/lisp/org/ox
c:/Home/.emacs.d/local/org-mode/lisp/ox-texinfo hides
c:/Local/Emacs/lisp/org/ox-texinfo
c:/Home/.emacs.d/local/org-mode/lisp/ox-publish hides
c:/Local/Emacs/lisp/org/ox-publish
c:/Home/.emacs.d/local/org-mode/lisp/ox-org hides
c:/Local/Emacs/lisp/org/ox-org
c:/Home/.emacs.d/local/org-mode/lisp/ox-odt hides
c:/Local/Emacs/lisp/org/ox-odt
c:/Home/.emacs.d/local/org-mode/lisp/ox-md hides
c:/Local/Emacs/lisp/org/ox-md
c:/Home/.emacs.d/local/org-mode/lisp/ox-man hides
c:/Local/Emacs/lisp/org/ox-man
c:/Home/.emacs.d/local/org-mode/lisp/ox-latex hides
c:/Local/Emacs/lisp/org/ox-latex
c:/Home/.emacs.d/local/org-mode/lisp/ox-icalendar hides
c:/Local/Emacs/lisp/org/ox-icalendar
c:/Home/.emacs.d/local/org-mode/lisp/ox-html hides
c:/Local/Emacs/lisp/org/ox-html
c:/Home/.emacs.d/local/org-mode/lisp/ox-beamer hides
c:/Local/Emacs/lisp/org/ox-beamer
c:/Home/.emacs.d/local/org-mode/lisp/ox-ascii hides
c:/Local/Emacs/lisp/org/ox-ascii
c:/Home/.emacs.d/local/org-mode/lisp/org hides c:/Local/Emacs/lisp/org/org
c:/Home/.emacs.d/local/org-mode/lisp/org-w3m hides
c:/Local/Emacs/lisp/org/org-w3m
c:/Home/.emacs.d/local/org-mode/lisp/org-version hides
c:/Local/Emacs/lisp/org/org-version
c:/Home/.emacs.d/local/org-mode/lisp/org-timer hides
c:/Local/Emacs/lisp/org/org-timer
c:/Home/.emacs.d/local/org-mode/lisp/org-table hides
c:/Local/Emacs/lisp/org/org-table
c:/Home/.emacs.d/local/org-mode/lisp/org-src hides
c:/Local/Emacs/lisp/org/org-src
c:/Home/.emacs.d/local/org-mode/lisp/org-rmail hides
c:/Local/Emacs/lisp/org/org-rmail
c:/Home/.emacs.d/local/org-mode/lisp/org-protocol hides
c:/Local/Emacs/lisp/org/org-protocol
c:/Home/.emacs.d/local/org-mode/lisp/org-plot hides
c:/Local/Emacs/lisp/org/org-plot
c:/Home/.emacs.d/local/org-mode/lisp/org-pcomplete hides
c:/Local/Emacs/lisp/org/org-pcomplete
c:/Home/.emacs.d/local/org-mode/lisp/org-mouse hides
c:/Local/Emacs/lisp/org/org-mouse
c:/Home/.emacs.d/local/org-mode/lisp/org-mobile hides
c:/Local/Emacs/lisp/org/org-mobile
c:/Home/.emacs.d/local/org-mode/lisp/org-mhe hides
c:/Local/Emacs/lisp/org/org-mhe
c:/Home/.emacs.d/local/org-mode/lisp/org-macs hides
c:/Local/Emacs/lisp/org/org-macs
c:/Home/.emacs.d/local/org-mode/lisp/org-macro hides
c:/Local/Emacs/lisp/org/org-macro
c:/Home/.emacs.d/local/org-mode/lisp/org-loaddefs hides
c:/Local/Emacs/lisp/org/org-loaddefs
c:/Home/.emacs.d/local/org-mode/lisp/org-list hides
c:/Local/Emacs/lisp/org/org-list
c:/Home/.emacs.d/local/org-mode/lisp/org-irc hides
c:/Local/Emacs/lisp/org/org-irc
c:/Home/.emacs.d/local/org-mode/lisp/org-install hides
c:/Local/Emacs/lisp/org/org-install
c:/Home/.emacs.d/local/org-mode/lisp/org-inlinetask hides
c:/Local/Emacs/lisp/org/org-inlinetask
c:/Home/.emacs.d/local/org-mode/lisp/org-info hides
c:/Local/Emacs/lisp/org/org-info
c:/Home/.emacs.d/local/org-mode/lisp/org-indent hides
c:/Local/Emacs/lisp/org/org-indent
c:/Home/.emacs.d/local/org-mode/lisp/org-id hides
c:/Local/Emacs/lisp/org/org-id
c:/Home/.emacs.d/local/org-mode/lisp/org-habit hides
c:/Local/Emacs/lisp/org/org-habit
c:/Home/.emacs.d/local/org-mode/lisp/org-gnus hides
c:/Local/Emacs/lisp/org/org-gnus
c:/Home/.emacs.d/local/org-mode/lisp/org-footnote hides
c:/Local/Emacs/lisp/org/org-footnote
c:/Home/.emacs.d/local/org-mode/lisp/org-feed hides
c:/Local/Emacs/lisp/org/org-feed
c:/Home/.emacs.d/local/org-mode/lisp/org-faces hides
c:/Local/Emacs/lisp/org/org-faces
c:/Home/.emacs.d/local/org-mode/lisp/org-eshell hides
c:/Local/Emacs/lisp/org/org-eshell
c:/Home/.emacs.d/local/org-mode/lisp/org-entities hides
c:/Local/Emacs/lisp/org/org-entities
c:/Home/.emacs.d/local/org-mode/lisp/org-element hides
c:/Local/Emacs/lisp/org/org-element
c:/Home/.emacs.d/local/org-mode/lisp/org-docview hides
c:/Local/Emacs/lisp/org/org-docview
c:/Home/.emacs.d/local/org-mode/lisp/org-datetree hides
c:/Local/Emacs/lisp/org/org-datetree
c:/Home/.emacs.d/local/org-mode/lisp/org-ctags hides
c:/Local/Emacs/lisp/org/org-ctags
c:/Home/.emacs.d/local/org-mode/lisp/org-crypt hides
c:/Local/Emacs/lisp/org/org-crypt
c:/Home/.emacs.d/local/org-mode/lisp/org-compat hides
c:/Local/Emacs/lisp/org/org-compat
c:/Home/.emacs.d/local/org-mode/lisp/org-colview hides
c:/Local/Emacs/lisp/org/org-colview
c:/Home/.emacs.d/local/org-mode/lisp/org-clock hides
c:/Local/Emacs/lisp/org/org-clock
c:/Home/.emacs.d/local/org-mode/lisp/org-capture hides
c:/Local/Emacs/lisp/org/org-capture
c:/Home/.emacs.d/local/org-mode/lisp/org-bibtex hides
c:/Local/Emacs/lisp/org/org-bibtex
c:/Home/.emacs.d/local/org-mode/lisp/org-bbdb hides
c:/Local/Emacs/lisp/org/org-bbdb
c:/Home/.emacs.d/local/org-mode/lisp/org-attach hides
c:/Local/Emacs/lisp/org/org-attach
c:/Home/.emacs.d/local/org-mode/lisp/org-archive hides
c:/Local/Emacs/lisp/org/org-archive
c:/Home/.emacs.d/local/org-mode/lisp/org-agenda hides
c:/Local/Emacs/lisp/org/org-agenda
c:/Home/.emacs.d/local/org-mode/lisp/ob hides c:/Local/Emacs/lisp/org/ob
c:/Home/.emacs.d/local/org-mode/lisp/ob-tangle hides
c:/Local/Emacs/lisp/org/ob-tangle
c:/Home/.emacs.d/local/org-mode/lisp/ob-table hides
c:/Local/Emacs/lisp/org/ob-table
c:/Home/.emacs.d/local/org-mode/lisp/ob-sqlite hides
c:/Local/Emacs/lisp/org/ob-sqlite
c:/Home/.emacs.d/local/org-mode/lisp/ob-sql hides
c:/Local/Emacs/lisp/org/ob-sql
c:/Home/.emacs.d/local/org-mode/lisp/ob-shen hides
c:/Local/Emacs/lisp/org/ob-shen
c:/Home/.emacs.d/local/org-mode/lisp/ob-screen hides
c:/Local/Emacs/lisp/org/ob-screen
c:/Home/.emacs.d/local/org-mode/lisp/ob-scheme hides
c:/Local/Emacs/lisp/org/ob-scheme
c:/Home/.emacs.d/local/org-mode/lisp/ob-scala hides
c:/Local/Emacs/lisp/org/ob-scala
c:/Home/.emacs.d/local/org-mode/lisp/ob-sass hides
c:/Local/Emacs/lisp/org/ob-sass
c:/Home/.emacs.d/local/org-mode/lisp/ob-ruby hides
c:/Local/Emacs/lisp/org/ob-ruby
c:/Home/.emacs.d/local/org-mode/lisp/ob-ref hides
c:/Local/Emacs/lisp/org/ob-ref
c:/Home/.emacs.d/local/org-mode/lisp/ob-R hides c:/Local/Emacs/lisp/org/ob-R
c:/Home/.emacs.d/local/org-mode/lisp/ob-python hides
c:/Local/Emacs/lisp/org/ob-python
c:/Home/.emacs.d/local/org-mode/lisp/ob-plantuml hides
c:/Local/Emacs/lisp/org/ob-plantuml
c:/Home/.emacs.d/local/org-mode/lisp/ob-picolisp hides
c:/Local/Emacs/lisp/org/ob-picolisp
c:/Home/.emacs.d/local/org-mode/lisp/ob-perl hides
c:/Local/Emacs/lisp/org/ob-perl
c:/Home/.emacs.d/local/org-mode/lisp/ob-org hides
c:/Local/Emacs/lisp/org/ob-org
c:/Home/.emacs.d/local/org-mode/lisp/ob-octave hides
c:/Local/Emacs/lisp/org/ob-octave
c:/Home/.emacs.d/local/org-mode/lisp/ob-ocaml hides
c:/Local/Emacs/lisp/org/ob-ocaml
c:/Home/.emacs.d/local/org-mode/lisp/ob-mscgen hides
c:/Local/Emacs/lisp/org/ob-mscgen
c:/Home/.emacs.d/local/org-mode/lisp/ob-maxima hides
c:/Local/Emacs/lisp/org/ob-maxima
c:/Home/.emacs.d/local/org-mode/lisp/ob-matlab hides
c:/Local/Emacs/lisp/org/ob-matlab
c:/Home/.emacs.d/local/org-mode/lisp/ob-makefile hides
c:/Local/Emacs/lisp/org/ob-makefile
c:/Home/.emacs.d/local/org-mode/lisp/ob-lob hides
c:/Local/Emacs/lisp/org/ob-lob
c:/Home/.emacs.d/local/org-mode/lisp/ob-lisp hides
c:/Local/Emacs/lisp/org/ob-lisp
c:/Home/.emacs.d/local/org-mode/lisp/ob-lilypond hides
c:/Local/Emacs/lisp/org/ob-lilypond
c:/Home/.emacs.d/local/org-mode/lisp/ob-ledger hides
c:/Local/Emacs/lisp/org/ob-ledger
c:/Home/.emacs.d/local/org-mode/lisp/ob-latex hides
c:/Local/Emacs/lisp/org/ob-latex
c:/Home/.emacs.d/local/org-mode/lisp/ob-keys hides
c:/Local/Emacs/lisp/org/ob-keys
c:/Home/.emacs.d/local/org-mode/lisp/ob-js hides
c:/Local/Emacs/lisp/org/ob-js
c:/Home/.emacs.d/local/org-mode/lisp/ob-java hides
c:/Local/Emacs/lisp/org/ob-java
c:/Home/.emacs.d/local/org-mode/lisp/ob-io hides
c:/Local/Emacs/lisp/org/ob-io
c:/Home/.emacs.d/local/org-mode/lisp/ob-haskell hides
c:/Local/Emacs/lisp/org/ob-haskell
c:/Home/.emacs.d/local/org-mode/lisp/ob-gnuplot hides
c:/Local/Emacs/lisp/org/ob-gnuplot
c:/Home/.emacs.d/local/org-mode/lisp/ob-fortran hides
c:/Local/Emacs/lisp/org/ob-fortran
c:/Home/.emacs.d/local/org-mode/lisp/ob-exp hides
c:/Local/Emacs/lisp/org/ob-exp
c:/Home/.emacs.d/local/org-mode/lisp/ob-eval hides
c:/Local/Emacs/lisp/org/ob-eval
c:/Home/.emacs.d/local/org-mode/lisp/ob-emacs-lisp hides
c:/Local/Emacs/lisp/org/ob-emacs-lisp
c:/Home/.emacs.d/local/org-mode/lisp/ob-dot hides
c:/Local/Emacs/lisp/org/ob-dot
c:/Home/.emacs.d/local/org-mode/lisp/ob-ditaa hides
c:/Local/Emacs/lisp/org/ob-ditaa
c:/Home/.emacs.d/local/org-mode/lisp/ob-css hides
c:/Local/Emacs/lisp/org/ob-css
c:/Home/.emacs.d/local/org-mode/lisp/ob-core hides
c:/Local/Emacs/lisp/org/ob-core
c:/Home/.emacs.d/local/org-mode/lisp/ob-comint hides
c:/Local/Emacs/lisp/org/ob-comint
c:/Home/.emacs.d/local/org-mode/lisp/ob-clojure hides
c:/Local/Emacs/lisp/org/ob-clojure
c:/Home/.emacs.d/local/org-mode/lisp/ob-calc hides
c:/Local/Emacs/lisp/org/ob-calc
c:/Home/.emacs.d/local/org-mode/lisp/ob-C hides c:/Local/Emacs/lisp/org/ob-C
c:/Home/.emacs.d/local/org-mode/lisp/ob-awk hides
c:/Local/Emacs/lisp/org/ob-awk
c:/Home/.emacs.d/local/org-mode/lisp/ob-asymptote hides
c:/Local/Emacs/lisp/org/ob-asymptote
c:/Home/.emacs.d/elpa/seq-2.19/seq hides c:/Local/Emacs/lisp/emacs-lisp/seq

Features:
(shadow sort mail-extr emacsbug sendmail misearch multi-isearch
network-stream nsm starttls texmathp debug smex two-column iso-transl
popwin latexenc reftex-auc preview prv-emacs tex-buf pdf-occur tablist
tablist-filter semantic/wisent/comp semantic/wisent
semantic/wisent/wisent semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local cedet pdf-isearch
let-alist pdf-misc font-latex view reftex-global reftex-parse reftex-dcr
org-eldoc org-indent company-jedi yasnippet highlight-indentation
flymake elpy pyvenv elpy-refactor files-x smartparens jedi-core
python-environment epc ctable concurrent deferred company-statistics
company-oddmuse company-keywords company-etags company-gtags
company-dabbrev-code company-dabbrev company-files company-cmake
company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb company python
autoconf autoconf-mode sh-script executable rainbow-delimiters undo-tree
diff anzu projectile grep ibuf-ext ibuffer make-mode desktop frameset
ox-bibtex reftex-cite reftex reftex-vars pdf-tools pdf-view pdf-cache
pdf-info tq pdf-util ob-shell ob-gnuplot ob-python ob-org ob-dot ob-lisp
ob-latex ox-koma-letter ox-beamer ox-reveal ox-odt ox-latex ox-icalendar
ox-html table ox-ascii ox-publish ox org-wl org-toc org-timer org-clock
org-screen term ehelp org-protocol org-panel org-mouse org-mime
org-interactive-query org-inlinetask org-info org-habit org-expiry
org-eval org-drill org-learn org-id hi-lock org-contacts org-capture
org-agenda gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source
utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win gnus
gnus-ems nnheader org-checklist org-bibtex bibtex org-annotate-file
web-mode caml tuareg_indent tuareg speedbar sb-image ezimage dframe smie
caml-help caml-types caml-emacs magic-latex-buffer iimage tex-mode latex
tex-ispell tex-style tex fp-specif slime-fancy slime-trace-dialog
slime-fontifying-fu slime-package-fu slime-references
slime-compiler-notes-tree slime-scratch slime-presentations bridge
slime-macrostep macrostep slime-mdot-fu slime-enclosing-context
slime-fuzzy slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl elp slime-parse slime
lisp-mnt gud apropos arc-mode archive-mode hyperspec browse-url
fp-pseudocode pascal rng-nxml rng-valid rng-loc rng-uri rng-parse
nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns nxml-mode
nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok ediprolog
writegood-mode winner windmove whitespace which-key which-func
volatile-highlights sumatra-forward ssh smart-mode-line rich-minority
saveplace savehist graphene-meta-theme sanityinc-tomorrow-night-theme
color-theme-sanityinc-tomorrow recentf tree-widget ranger ps-print
ps-def lpr popup page-break-lines ov outshine outshine-org-cmds outorg
org-element avl-tree org org-macro org-footnote org-pcomplete org-list
org-faces org-entities org-version ob-emacs-lisp ob ob-tangle org-src
ob-ref ob-lob ob-table ob-keys ob-exp ob-comint ob-core ob-eval
org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs noutline
outline operate-on-number calc-bin calc-ext calc calc-loaddefs calc-macs
oauth2 warnings plstore move-text midnight memory-usage magithub
magithub-ci magithub-issue magithub-cache magithub-core magit-blame
magit-stash magit-bisect magit-remote magit-commit magit-sequence magit
magit-apply magit-wip magit-log magit-diff smerge-mode magit-core
magit-autorevert magit-process magit-popup magit-mode magit-git crm
magit-section magit-utils git-commit log-edit message rfc822 mml mml-sec
epg mm-decode mm-bodies mm-encode mailabbrev mail-utils gmm-utils
mailheader pcvs-util add-log with-editor async-bytecomp async tramp-sh
tramp tramp-compat tramp-loaddefs trampver shell pcomplete server
lua-mode key-chord js2-mode etags xref project js sgml-mode cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs imenu-anywhere imenu ido-hacks flx-ido flx ido-vertical-mode
ido-ubiquitous ido-completing-read+ cus-edit cus-start cus-load wid-edit
ido hungry-delete htmlize hl-line highlight-symbol guru-mode grizzl
god-mode gitignore-mode gitconfig-mode conf-mode git-timemachine vc-git
gist gh-gist gh-oauth gh-api logito gh-cache pcache gh-auth gh-url
url-http tls gnutls url url-proxy url-privacy url-expand url-methods
url-history mailcap url-auth mail-parse rfc2231 rfc2047 rfc2045
ietf-drums url-cookie url-domsuf url-util url-gw timezone eieio-base
framemove flyspell ispell flycheck find-func epl ediff-merg ediff-wind
ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff easy-kill
thingatpt doc-view subr-x jka-compr image-mode discover-my-major makey
dired+ image-dired format-spec image-file dired-x dired-aux diff-hl
smartrep vc-dir ewoc vc vc-dispatcher diff-mode dictionary link
connection browse-kill-ring derived bookmark pp beacon autorevert
filenotify ag vc-svn compile comint ansi-color ring find-dired dired cl
edmacro kmacro cua-base delsel use-package diminish bind-key easy-mmode
finder-inf tex-site gh-common gh-profile url-parse auth-source gnus-util
mm-util help-fns mail-prsvr password-cache url-vars rx s ucs-normalize
marshal eieio-compat cl-seq ht json map dash eieio eieio-core cl-macs
advice info package epg-config seq byte-opt gv bytecomp byte-compile
cl-extra help-mode easymenu cconv cl-loaddefs pcase cl-lib time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table w32-win w32-vars
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
w32notify w32 multi-tty make-network-process emacs)

Memory information:
((conses 16 1899150 677834)
 (symbols 56 98781 0)
 (miscs 48 71108 15295)
 (strings 32 352119 167999)
 (string-bytes 1 10705287)
 (vectors 16 174375)
 (vector-slots 8 4203177 425963)
 (floats 8 16169 10128)
 (intervals 56 128230 8752)
 (buffers 976 80))

[-- Attachment #2: Type: text/html, Size: 27864 bytes --]

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 551 bytes --]

On 2016-11-25 01:24, Fabrice Popineau wrote:
> The python shell name is not passed to any underlying shell.
> It is used to create a process, so it must not be quoted in anyway.

Are you sure? Looking at the code, I see this:

  (python-shell-make-comint
      (or cmd (python-shell-calculate-command)) …)

And python-shell-make-comint does this:

  (split-string-and-unquote cmd)

(ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?

Clément.
   


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Fri, 25 Nov 2016 02:03:38 -0500
> 
> On 2016-11-25 01:24, Fabrice Popineau wrote:
> > The python shell name is not passed to any underlying shell.
> > It is used to create a process, so it must not be quoted in anyway.
> 
> Are you sure? Looking at the code, I see this:
> 
>   (python-shell-make-comint
>       (or cmd (python-shell-calculate-command)) …)
> 
> And python-shell-make-comint does this:
> 
>   (split-string-and-unquote cmd)
> 
> (ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?

The quoting needs to be done only where a shell command is created
that is about to be passed to a shell.  I believe in this case the
quoting is done too early.

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1348 bytes --]

On 2016-11-25 03:32, Eli Zaretskii wrote:
>> From: Clément Pit--Claudel <clement.pit@gmail.com>
>> Date: Fri, 25 Nov 2016 02:03:38 -0500
>>
>> On 2016-11-25 01:24, Fabrice Popineau wrote:
>>> The python shell name is not passed to any underlying shell.
>>> It is used to create a process, so it must not be quoted in anyway.
>>
>> Are you sure? Looking at the code, I see this:
>>
>>   (python-shell-make-comint
>>       (or cmd (python-shell-calculate-command)) …)
>>
>> And python-shell-make-comint does this:
>>
>>   (split-string-and-unquote cmd)
>>
>> (ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?
> 
> The quoting needs to be done only where a shell command is created
> that is about to be passed to a shell.  I believe in this case the
> quoting is done too early.

Certainly; but it seems like we'll need to modify more than just python-shell-calculate-command; in fact, we probably could remove it entirely: there's no need to glue together the command name and its arguments before passing them to comint, is there?

I was just pointing out that Fabrice's solution of writing

  (format "%s %s"
          python-shell-interpreter
          python-shell-interpreter-args))

would probably break things.  Won't it?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

merge 25025 20744
quit

On Fri, Nov 25, 2016 at 3:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Clément Pit--Claudel <clement.pit@gmail.com>
>> Date: Fri, 25 Nov 2016 02:03:38 -0500
>>
>> On 2016-11-25 01:24, Fabrice Popineau wrote:
>> > The python shell name is not passed to any underlying shell.
>> > It is used to create a process, so it must not be quoted in anyway.
>>
>> Are you sure? Looking at the code, I see this:
>>
>>   (python-shell-make-comint
>>       (or cmd (python-shell-calculate-command)) …)
>>
>> And python-shell-make-comint does this:
>>
>>   (split-string-and-unquote cmd)
>>
>> (ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?
>
> The quoting needs to be done only where a shell command is created
> that is about to be passed to a shell.  I believe in this case the
> quoting is done too early.
>

I think the whole command should be quoted with
combine-and-quote-strings to balance the call to
split-string-and-unquote.

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 25 Nov 2016 09:59:43 -0500
> Cc: Clément Pit--Claudel <clement.pit@gmail.com>, 
> 	25025@debbugs.gnu.org
> 
> >> > The python shell name is not passed to any underlying shell.
> >> > It is used to create a process, so it must not be quoted in anyway.
> >>
> >> Are you sure? Looking at the code, I see this:
> >>
> >>   (python-shell-make-comint
> >>       (or cmd (python-shell-calculate-command)) …)
> >>
> >> And python-shell-make-comint does this:
> >>
> >>   (split-string-and-unquote cmd)
> >>
> >> (ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?
> >
> > The quoting needs to be done only where a shell command is created
> > that is about to be passed to a shell.  I believe in this case the
> > quoting is done too early.
> >
> 
> I think the whole command should be quoted with
> combine-and-quote-strings to balance the call to
> split-string-and-unquote.

Why not remove both of those calls?  Do you understand why they are
needed in that case?

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Sat, Nov 26, 2016 at 1:43 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 25 Nov 2016 09:59:43 -0500
>> Cc: Clément Pit--Claudel <clement.pit@gmail.com>,
>>       25025@debbugs.gnu.org
>>
>> >> > The python shell name is not passed to any underlying shell.
>> >> > It is used to create a process, so it must not be quoted in anyway.
>> >>
>> >> Are you sure? Looking at the code, I see this:
>> >>
>> >>   (python-shell-make-comint
>> >>       (or cmd (python-shell-calculate-command)) …)
>> >>
>> >> And python-shell-make-comint does this:
>> >>
>> >>   (split-string-and-unquote cmd)
>> >>
>> >> (ok, this is weird).  But still, if the command is "C:\Program Files\Python\python.exe", then we *do* need the shell quoting, right?
>> >
>> > The quoting needs to be done only where a shell command is created
>> > that is about to be passed to a shell.  I believe in this case the
>> > quoting is done too early.
>> >
>>
>> I think the whole command should be quoted with
>> combine-and-quote-strings to balance the call to
>> split-string-and-unquote.
>
> Why not remove both of those calls?  Do you understand why they are
> needed in that case?

Well, CMD can also come from user input, so we would need some way for
the user to specify a list of arguments. Currently that can work by
entering a string that would be split by split-string-and-unquote. It
might be more intuitive to actually use a shell and then the user
would enter a shell command (though inserting a shell into things
might bring more complications).

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 924 bytes --]

On 2016-11-26 19:50, Noam Postavsky wrote:
> Well, CMD can also come from user input, so we would need some way for
> the user to specify a list of arguments. Currently that can work by
> entering a string that would be split by split-string-and-unquote. It
> might be more intuitive to actually use a shell and then the user
> would enter a shell command (though inserting a shell into things
> might bring more complications).

On that topic, I feel that Emacs is missing a solid, standard thing like python's shlex.  split-string-and-unquote is entirely inadequate to split shell commands (it's a great tool, but isn't a mistake to use it on user-supplied command lines?):

  (split-string-and-unquote "python -c 'print \"a\"'")
  ⇒ ("python" "-c" "'print" "a" "'")

John's eshell has such parsing features, but I've never seen them used in other packages; is there a good reason?

Cheers,
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 26 Nov 2016 19:50:40 -0500
> Cc: Clément Pit--Claudel <clement.pit@gmail.com>, 
> 	25025@debbugs.gnu.org
> 
> >> I think the whole command should be quoted with
> >> combine-and-quote-strings to balance the call to
> >> split-string-and-unquote.
> >
> > Why not remove both of those calls?  Do you understand why they are
> > needed in that case?
> 
> Well, CMD can also come from user input, so we would need some way for
> the user to specify a list of arguments. Currently that can work by
> entering a string that would be split by split-string-and-unquote.

Why does it need to be split?  A shell command can (even should) be
handed to the shell as a single string.

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Sat, 26 Nov 2016 19:50:40 -0500
>> Cc: Clément Pit--Claudel <clement.pit@gmail.com>, 
>> 	25025@debbugs.gnu.org
>> 
>> >> I think the whole command should be quoted with
>> >> combine-and-quote-strings to balance the call to
>> >> split-string-and-unquote.
>> >
>> > Why not remove both of those calls?  Do you understand why they are
>> > needed in that case?
>> 
>> Well, CMD can also come from user input, so we would need some way for
>> the user to specify a list of arguments. Currently that can work by
>> entering a string that would be split by split-string-and-unquote.
>
> Why does it need to be split?  A shell command can (even should) be
> handed to the shell as a single string.

Currently it's not a shell command, because a shell isn't being used.
My other suggestion was to use a shell:

>> It might be more intuitive to actually use a shell and then the user
>> would enter a shell command (though inserting a shell into things
>> might bring more complications).

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
> Date: Sun, 27 Nov 2016 11:06:03 -0500
> 
> > Why does it need to be split?  A shell command can (even should) be
> > handed to the shell as a single string.
> 
> Currently it's not a shell command, because a shell isn't being used.
> My other suggestion was to use a shell:
> 
> >> It might be more intuitive to actually use a shell and then the user
> >> would enter a shell command (though inserting a shell into things
> >> might bring more complications).

If it doesn't use a shell, then it has no business quoting commands or
their parts using shell-related APIs.

So yes, I think using a shell would be TRT here.  Can someone please
work on a patch in that direction?  This problem exists for a long
time, so I hope we could solve it soon.

bug#25025: python-shell-calculate-command is wrong

[Andreas Röhler]

On 27.11.2016 17:12, Eli Zaretskii wrote:
>> From: npostavs@users.sourceforge.net
>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
>> Date: Sun, 27 Nov 2016 11:06:03 -0500
>>
>>> Why does it need to be split?  A shell command can (even should) be
>>> handed to the shell as a single string.
>> Currently it's not a shell command, because a shell isn't being used.
>> My other suggestion was to use a shell:
>>
>>>> It might be more intuitive to actually use a shell and then the user
>>>> would enter a shell command (though inserting a shell into things
>>>> might bring more complications).
> If it doesn't use a shell, then it has no business quoting commands or
> their parts using shell-related APIs.
>
> So yes, I think using a shell would be TRT here.  Can someone please
> work on a patch in that direction?  This problem exists for a long
> time, so I hope we could solve it soon.
>
>
>

Executing Python code via a shell might produce subtle bugs. See 
https://bugs.launchpad.net/python-mode/+bug/550661

Following the OP's suggestion seems reasonable. Just drop it at place 
and see if something gets broken - wouldn't expect it.

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
> Executing Python code via a shell might produce subtle bugs. See
> https://bugs.launchpad.net/python-mode/+bug/550661

We're talking about executing the python executable via a shell, not
python code via a "python shell", which is something different.

>
> Following the OP's suggestion seems reasonable. Just drop it at place
> and see if something gets broken - wouldn't expect it.

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#8 for why it
would break.

bug#25025: python-shell-calculate-command is wrong

[Andreas Röhler]

On 28.11.2016 15:15, npostavs@users.sourceforge.net wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>> Executing Python code via a shell might produce subtle bugs. See
>> https://bugs.launchpad.net/python-mode/+bug/550661
> We're talking about executing the python executable via a shell, not
> python code via a "python shell", which is something different.

Seeing the example was not clear, sorry. Just tried to you give a piece 
of code which would fail doing shell-commend-on-region.
Try print(u'\xA9') with python2. With python3 it works here. AFAIU the 
current python.el doesn't pipe to Python-code to exec through a shell - 
for related reasons.


>
>> Following the OP's suggestion seems reasonable. Just drop it at place
>> and see if something gets broken - wouldn't expect it.
> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#8 for why it
> would break.

Only see a question there, not a statement. BTW have no system of the 
OP's kind, can't check that.

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

>>> Following the OP's suggestion seems reasonable. Just drop it at place
>>> and see if something gets broken - wouldn't expect it.
>> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#8 for why it
>> would break.
>
> Only see a question there, not a statement. BTW have no system of the
> OP's kind, can't check that.

You don't need a Windows system, just imagine what would happen if
`python-shell-interpreter' had a space in it, and it wasn't quoted as
suggested in OP.

bug#25025: python-shell-calculate-command is wrong

[Andreas Röhler]

On 30.11.2016 01:39, npostavs@users.sourceforge.net wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>>>> Following the OP's suggestion seems reasonable. Just drop it at place
>>>> and see if something gets broken - wouldn't expect it.
>>> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#8 for why it
>>> would break.
>> Only see a question there, not a statement. BTW have no system of the
>> OP's kind, can't check that.
> You don't need a Windows system, just imagine what would happen if
> `python-shell-interpreter' had a space in it, and it wasn't quoted as
> suggested in OP.

An executable with a space in its name? Hmm, didn't notice such thing.

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 163 bytes --]

On 2016-11-30 01:39, Andreas Röhler wrote:
> An executable with a space in its name? Hmm, didn't notice such thing.

"C:\Program Files\Python\python.exe" ?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
>> Date: Sun, 27 Nov 2016 11:06:03 -0500
>> 
>> > Why does it need to be split?  A shell command can (even should) be
>> > handed to the shell as a single string.
>> 
>> Currently it's not a shell command, because a shell isn't being used.
>> My other suggestion was to use a shell:
>> 
>> >> It might be more intuitive to actually use a shell and then the user
>> >> would enter a shell command (though inserting a shell into things
>> >> might bring more complications).
>
> If it doesn't use a shell, then it has no business quoting commands or
> their parts using shell-related APIs.
>
> So yes, I think using a shell would be TRT here.  Can someone please
> work on a patch in that direction?  This problem exists for a long
> time, so I hope we could solve it soon.

Hmm, the difficulty in using a shell is that the current code wants to
parse the command into interpreter and arguments in order to match
against `python-shell-completion-native-disabled-interpreters'.

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1464 bytes --]

On 2016-11-29 19:36, npostavs@users.sourceforge.net wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> From: npostavs@users.sourceforge.net
>>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
>>> Date: Sun, 27 Nov 2016 11:06:03 -0500
>>>
>>>> Why does it need to be split?  A shell command can (even should) be
>>>> handed to the shell as a single string.
>>>
>>> Currently it's not a shell command, because a shell isn't being used.
>>> My other suggestion was to use a shell:
>>>
>>>>> It might be more intuitive to actually use a shell and then the user
>>>>> would enter a shell command (though inserting a shell into things
>>>>> might bring more complications).
>>
>> If it doesn't use a shell, then it has no business quoting commands or
>> their parts using shell-related APIs.
>>
>> So yes, I think using a shell would be TRT here.  Can someone please
>> work on a patch in that direction?  This problem exists for a long
>> time, so I hope we could solve it soon.
> 
> Hmm, the difficulty in using a shell is that the current code wants to
> parse the command into interpreter and arguments in order to match
> against `python-shell-completion-native-disabled-interpreters'.

That doesn't prevent us from using a shell.  We run the command unmodified through a shell, and we split it and analyze it separately to decide whether to enable completion.  But we don't split and reassemble it before running it.

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Clément Pit--Claudel <clement.pit@gmail.com> writes:

> On 2016-11-29 19:36, npostavs@users.sourceforge.net wrote:
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>>>> From: npostavs@users.sourceforge.net
>>>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
>>>> Date: Sun, 27 Nov 2016 11:06:03 -0500
>>>>
>>>>> Why does it need to be split?  A shell command can (even should) be
>>>>> handed to the shell as a single string.
>>>>
>>>> Currently it's not a shell command, because a shell isn't being used.
>>>> My other suggestion was to use a shell:
>>>>
>>>>>> It might be more intuitive to actually use a shell and then the user
>>>>>> would enter a shell command (though inserting a shell into things
>>>>>> might bring more complications).
>>>
>>> If it doesn't use a shell, then it has no business quoting commands or
>>> their parts using shell-related APIs.
>>>
>>> So yes, I think using a shell would be TRT here.  Can someone please
>>> work on a patch in that direction?  This problem exists for a long
>>> time, so I hope we could solve it soon.
>> 
>> Hmm, the difficulty in using a shell is that the current code wants to
>> parse the command into interpreter and arguments in order to match
>> against `python-shell-completion-native-disabled-interpreters'.
>
> That doesn't prevent us from using a shell.  We run the command
> unmodified through a shell, and we split it and analyze it separately
> to decide whether to enable completion.  But we don't split and
> reassemble it before running it.

Yes, but then we need to parse a shell quoted command, which is quite a
bit more difficult.  That gets back to your suggestion about getting an
elisp equivalent to shlex I suppose.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#28

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: Eli Zaretskii <eliz@gnu.org>,  25025@debbugs.gnu.org
> Date: Tue, 29 Nov 2016 20:56:16 -0500
> 
> Clément Pit--Claudel <clement.pit@gmail.com> writes:
> 
> > On 2016-11-29 19:36, npostavs@users.sourceforge.net wrote:
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >>>> From: npostavs@users.sourceforge.net
> >>>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
> >>>> Date: Sun, 27 Nov 2016 11:06:03 -0500
> >>>>
> >>>>> Why does it need to be split?  A shell command can (even should) be
> >>>>> handed to the shell as a single string.
> >>>>
> >>>> Currently it's not a shell command, because a shell isn't being used.
> >>>> My other suggestion was to use a shell:
> >>>>
> >>>>>> It might be more intuitive to actually use a shell and then the user
> >>>>>> would enter a shell command (though inserting a shell into things
> >>>>>> might bring more complications).
> >>>
> >>> If it doesn't use a shell, then it has no business quoting commands or
> >>> their parts using shell-related APIs.
> >>>
> >>> So yes, I think using a shell would be TRT here.  Can someone please
> >>> work on a patch in that direction?  This problem exists for a long
> >>> time, so I hope we could solve it soon.
> >> 
> >> Hmm, the difficulty in using a shell is that the current code wants to
> >> parse the command into interpreter and arguments in order to match
> >> against `python-shell-completion-native-disabled-interpreters'.
> >
> > That doesn't prevent us from using a shell.  We run the command
> > unmodified through a shell, and we split it and analyze it separately
> > to decide whether to enable completion.  But we don't split and
> > reassemble it before running it.
> 
> Yes, but then we need to parse a shell quoted command, which is quite a
> bit more difficult.  That gets back to your suggestion about getting an
> elisp equivalent to shlex I suppose.
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#28

Confused: how does python-shell-completion-native-disabled-interpreters
get into this picture?  The function which uses it,
python-shell-completion-native-interpreter-disabled-p, looks at
python-shell-interpreter, which isn't affected by quoting or by how
the command is treated.  What am I missing?

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Wed, Nov 30, 2016 at 10:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: npostavs@users.sourceforge.net
>> Cc: Eli Zaretskii <eliz@gnu.org>,  25025@debbugs.gnu.org
>> Date: Tue, 29 Nov 2016 20:56:16 -0500
>>
>> Clément Pit--Claudel <clement.pit@gmail.com> writes:
>>
>> > On 2016-11-29 19:36, npostavs@users.sourceforge.net wrote:
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >>
>> >>>> From: npostavs@users.sourceforge.net
>> >>>> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
>> >>>> Date: Sun, 27 Nov 2016 11:06:03 -0500
>> >>>>
>> >>>>> Why does it need to be split?  A shell command can (even should) be
>> >>>>> handed to the shell as a single string.
>> >>>>
>> >>>> Currently it's not a shell command, because a shell isn't being used.
>> >>>> My other suggestion was to use a shell:
>> >>>>
>> >>>>>> It might be more intuitive to actually use a shell and then the user
>> >>>>>> would enter a shell command (though inserting a shell into things
>> >>>>>> might bring more complications).
>> >>>
>> >>> If it doesn't use a shell, then it has no business quoting commands or
>> >>> their parts using shell-related APIs.
>> >>>
>> >>> So yes, I think using a shell would be TRT here.  Can someone please
>> >>> work on a patch in that direction?  This problem exists for a long
>> >>> time, so I hope we could solve it soon.
>> >>
>> >> Hmm, the difficulty in using a shell is that the current code wants to
>> >> parse the command into interpreter and arguments in order to match
>> >> against `python-shell-completion-native-disabled-interpreters'.
>> >
>> > That doesn't prevent us from using a shell.  We run the command
>> > unmodified through a shell, and we split it and analyze it separately
>> > to decide whether to enable completion.  But we don't split and
>> > reassemble it before running it.
>>
>> Yes, but then we need to parse a shell quoted command, which is quite a
>> bit more difficult.  That gets back to your suggestion about getting an
>> elisp equivalent to shlex I suppose.
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25025#28
>
> Confused: how does python-shell-completion-native-disabled-interpreters
> get into this picture?  The function which uses it,
> python-shell-completion-native-interpreter-disabled-p, looks at
> python-shell-interpreter, which isn't affected by quoting or by how
> the command is treated.  What am I missing?

This?

(defun run-python (&optional cmd dedicated show)
  ...
   (python-shell-make-comint
    (or cmd (python-shell-calculate-command))
  ...

(defun python-shell-make-comint (cmd proc-name &optional show internal)
  ...
          (let* ((cmdlist (split-string-and-unquote cmd))
                 (interpreter (car cmdlist))
                 (args (cdr cmdlist))
                 (buffer (apply #'make-comint-in-buffer proc-name
proc-buffer-name
                                interpreter nil args))
                 ...
                 ;; Users can override the interpreter and args
                 ;; interactively when calling `run-python', let-binding
                 ;; these allows having the new right values in all
                 ;; setup code that is done in `inferior-python-mode',
                 ;; which is important, especially for prompt detection.
                 (python-shell--interpreter interpreter)
                 (python-shell--interpreter-args
                  (mapconcat #'identity args " ")))
            (with-current-buffer buffer
              (inferior-python-mode))
  ...

(define-derived-mode inferior-python-mode comint-mode "Inferior Python"
  ...
  ;; Users can interactively override default values for
  ;; `python-shell-interpreter' and `python-shell-interpreter-args'
  ;; when calling `run-python'.  This ensures values let-bound in
  ;; `python-shell-make-comint' are locally set if needed.
  (set (make-local-variable 'python-shell-interpreter)
       (or python-shell--interpreter python-shell-interpreter))
  (set (make-local-variable 'python-shell-interpreter-args)
       (or python-shell--interpreter-args python-shell-interpreter-args))
  ...

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 30 Nov 2016 17:10:34 -0500
> Cc: Clément Pit--Claudel <clement.pit@gmail.com>, 
> 	25025@debbugs.gnu.org
> 
> > Confused: how does python-shell-completion-native-disabled-interpreters
> > get into this picture?  The function which uses it,
> > python-shell-completion-native-interpreter-disabled-p, looks at
> > python-shell-interpreter, which isn't affected by quoting or by how
> > the command is treated.  What am I missing?
> 
> This?
> 
> (defun run-python (&optional cmd dedicated show)
>   ...
>    (python-shell-make-comint
>     (or cmd (python-shell-calculate-command))
>   ...
> 
> (defun python-shell-make-comint (cmd proc-name &optional show internal)
>   ...
>           (let* ((cmdlist (split-string-and-unquote cmd))
>                  (interpreter (car cmdlist))
>                  (args (cdr cmdlist))
>                  (buffer (apply #'make-comint-in-buffer proc-name
> proc-buffer-name
>                                 interpreter nil args))
>                  ...
>                  ;; Users can override the interpreter and args
>                  ;; interactively when calling `run-python', let-binding
>                  ;; these allows having the new right values in all
>                  ;; setup code that is done in `inferior-python-mode',
>                  ;; which is important, especially for prompt detection.
>                  (python-shell--interpreter interpreter)
>                  (python-shell--interpreter-args
>                   (mapconcat #'identity args " ")))
>             (with-current-buffer buffer
>               (inferior-python-mode))
>   ...
> 
> (define-derived-mode inferior-python-mode comint-mode "Inferior Python"
>   ...
>   ;; Users can interactively override default values for
>   ;; `python-shell-interpreter' and `python-shell-interpreter-args'
>   ;; when calling `run-python'.  This ensures values let-bound in
>   ;; `python-shell-make-comint' are locally set if needed.
>   (set (make-local-variable 'python-shell-interpreter)
>        (or python-shell--interpreter python-shell-interpreter))
>   (set (make-local-variable 'python-shell-interpreter-args)
>        (or python-shell--interpreter-args python-shell-interpreter-args))
>   ...

Thanks.

So the problematic scenario is that the user sets
python-shell-interpreter to something we already checked, and then
manually feeds us with a command that invokes a different Python
interpreter, is that it?  Is that a frequent use case, which we
should care about?  Just to produce a warning?

Even if we do want to care about, we could try matching the beginning
of the command, perhaps after an optional quote character, and if we
don't recognize the interpreter, do nothing.  Would that be
sufficient?

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:
>
> So the problematic scenario is that the user sets
> python-shell-interpreter to something we already checked, and then
> manually feeds us with a command that invokes a different Python
> interpreter, is that it?  Is that a frequent use case, which we
> should care about?  Just to produce a warning?
>
> Even if we do want to care about, we could try matching the beginning
> of the command, perhaps after an optional quote character, and if we
> don't recognize the interpreter, do nothing.  Would that be
> sufficient?

To be honest, I don't use python enough to say what the best fix is
here.  I can say that the easiest way to solve this bug is:

--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2379,7 +2379,7 @@ python-shell-internal-get-process-name
 (defun python-shell-calculate-command ()
   "Calculate the string used to execute the inferior Python process."
   (format "%s %s"
-          (shell-quote-argument python-shell-interpreter)
+          (combine-and-quote-strings (list python-shell-interpreter))
           python-shell-interpreter-args))

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
> Date: Thu, 01 Dec 2016 20:12:13 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >
> > So the problematic scenario is that the user sets
> > python-shell-interpreter to something we already checked, and then
> > manually feeds us with a command that invokes a different Python
> > interpreter, is that it?  Is that a frequent use case, which we
> > should care about?  Just to produce a warning?
> >
> > Even if we do want to care about, we could try matching the beginning
> > of the command, perhaps after an optional quote character, and if we
> > don't recognize the interpreter, do nothing.  Would that be
> > sufficient?
> 
> To be honest, I don't use python enough to say what the best fix is
> here.

Me neither.

> I can say that the easiest way to solve this bug is:
> 
> --- a/lisp/progmodes/python.el
> +++ b/lisp/progmodes/python.el
> @@ -2379,7 +2379,7 @@ python-shell-internal-get-process-name
>  (defun python-shell-calculate-command ()
>    "Calculate the string used to execute the inferior Python process."
>    (format "%s %s"
> -          (shell-quote-argument python-shell-interpreter)
> +          (combine-and-quote-strings (list python-shell-interpreter))
>            python-shell-interpreter-args))

Isn't combine-and-quote-strings wrong for quoting shell commands?
AFAIR, it doesn't DTRT with some special characters that can appear in
file names on Unix.  Am I mistaken?

But if my fears are unjustified, sure, why not?  Clément, WDYT?

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Fri, Dec 2, 2016 at 2:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> --- a/lisp/progmodes/python.el
>> +++ b/lisp/progmodes/python.el
>> @@ -2379,7 +2379,7 @@ python-shell-internal-get-process-name
>>  (defun python-shell-calculate-command ()
>>    "Calculate the string used to execute the inferior Python process."
>>    (format "%s %s"
>> -          (shell-quote-argument python-shell-interpreter)
>> +          (combine-and-quote-strings (list python-shell-interpreter))
>>            python-shell-interpreter-args))
>
> Isn't combine-and-quote-strings wrong for quoting shell commands?
> AFAIR, it doesn't DTRT with some special characters that can appear in
> file names on Unix.  Am I mistaken?

It's not a shell command though, hence this bug.

>
> But if my fears are unjustified, sure, why not?  Clément, WDYT?

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 2 Dec 2016 09:16:04 -0500
> Cc: 25025@debbugs.gnu.org, Clément Pit--Claudel <clement.pit@gmail.com>
> 
> On Fri, Dec 2, 2016 at 2:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> --- a/lisp/progmodes/python.el
> >> +++ b/lisp/progmodes/python.el
> >> @@ -2379,7 +2379,7 @@ python-shell-internal-get-process-name
> >>  (defun python-shell-calculate-command ()
> >>    "Calculate the string used to execute the inferior Python process."
> >>    (format "%s %s"
> >> -          (shell-quote-argument python-shell-interpreter)
> >> +          (combine-and-quote-strings (list python-shell-interpreter))
> >>            python-shell-interpreter-args))
> >
> > Isn't combine-and-quote-strings wrong for quoting shell commands?
> > AFAIR, it doesn't DTRT with some special characters that can appear in
> > file names on Unix.  Am I mistaken?
> 
> It's not a shell command though, hence this bug.

No, but the code in question generates a shell command, from the file
name of the interpreter and its arguments.  Right?

bug#25025: python-shell-calculate-command is wrong

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 2 Dec 2016 09:16:04 -0500
>> Cc: 25025@debbugs.gnu.org, Clément Pit--Claudel <clement.pit@gmail.com>
>> 
>> On Fri, Dec 2, 2016 at 2:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> --- a/lisp/progmodes/python.el
>> >> +++ b/lisp/progmodes/python.el
>> >> @@ -2379,7 +2379,7 @@ python-shell-internal-get-process-name
>> >>  (defun python-shell-calculate-command ()
>> >>    "Calculate the string used to execute the inferior Python process."
>> >>    (format "%s %s"
>> >> -          (shell-quote-argument python-shell-interpreter)
>> >> +          (combine-and-quote-strings (list python-shell-interpreter))
>> >>            python-shell-interpreter-args))
>> >
>> > Isn't combine-and-quote-strings wrong for quoting shell commands?
>> > AFAIR, it doesn't DTRT with some special characters that can appear in
>> > file names on Unix.  Am I mistaken?
>> 
>> It's not a shell command though, hence this bug.
>
> No, but the code in question generates a shell command, from the file
> name of the interpreter and its arguments.  Right?

Okay, let me rephrase.  `python-shell-calculate-command' currently
generates a shell command, but none of its callers treat the result as a
shell command (they don't pass it to a shell, they parse it with
`split-string-and-unquote').  Therefore, the easiest fix is to change
`python-shell-calculate-command' to no longer generate a shell command.

The other possiblity is to change the callers to treat
`python-shell-calculate-command's result as a shell command, but that
looks more difficult (though it may be the better solution overall).

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 25025@debbugs.gnu.org,  clement.pit@gmail.com
> Date: Fri, 02 Dec 2016 10:07:57 -0500
> 
> Okay, let me rephrase.  `python-shell-calculate-command' currently
> generates a shell command, but none of its callers treat the result as a
> shell command (they don't pass it to a shell, they parse it with
> `split-string-and-unquote').  Therefore, the easiest fix is to change
> `python-shell-calculate-command' to no longer generate a shell command.

Fine with me (and maybe also change the function's name while you are
at it).

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 2183 bytes --]

On 2016-12-02 02:35, Eli Zaretskii wrote:
> Isn't combine-and-quote-strings wrong for quoting shell commands?
> AFAIR, it doesn't DTRT with some special characters that can appear in
> file names on Unix.  Am I mistaken?
> 
> But if my fears are unjustified, sure, why not?  Clément, WDYT?

On 2016-12-02 10:07, npostavs@users.sourceforge.net wrote:
> Okay, let me rephrase.  `python-shell-calculate-command' currently
> generates a shell command, but none of its callers treat the result as a
> shell command (they don't pass it to a shell, they parse it with
> `split-string-and-unquote').  Therefore, the easiest fix is to change
> `python-shell-calculate-command' to no longer generate a shell command.
> 
> The other possiblity is to change the callers to treat
> `python-shell-calculate-command's result as a shell command, but that
> looks more difficult (though it may be the better solution overall).

Currently, run-python can read a shell command; do we want to remove this feature? If not, then we do need a shell, don't we?

As far as I understand we have two conflicting requirements:

* One part of the code wants access to switches passed to python, as a list of switches.
* One part of the code wants to read a python command, including switches, from the user.

I'm not sure that we can get these two to both work in all cases, unless we come up with a robust way to parse shell commands given by the user.  I see multiple solutions:

1. Use a shell to run python. Then the part of the code that wants to know which switches are being passed can use the possibly-incorrect split-string-and-unquote to split user-supplied strings, but the user-supplied command is run as-is through a shell.

2. Keep running python as a subprocess, without a shell; in that case, user-supplied commands (in C-u M-x run-python) need to be "parsed" back into command + switches before running them, which introduces a small potential for incorrect parsing.

Noam, your approach is (2), right?  I like the simplicity.

In the long run, it would be nice to offer a read-shell-command-as-list function, probably based on eshell.

Cheers,
Clément.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Fri, Dec 2, 2016 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> Fine with me (and maybe also change the function's name while you are
> at it).

If you meant to remove the "shell" from
`python-shell-calculate-command', I think that refers to the "python
shell" (which would be called REPL in Lisp speak). There are quite a
few other functions and variables with the python-shell prefix.

On Fri, Dec 2, 2016 at 11:15 AM, Clément Pit--Claudel
<clement.pit@gmail.com> wrote:
> On 2016-12-02 02:35, Eli Zaretskii wrote:
>> Isn't combine-and-quote-strings wrong for quoting shell commands?
>> AFAIR, it doesn't DTRT with some special characters that can appear in
>> file names on Unix.  Am I mistaken?
>>
>> But if my fears are unjustified, sure, why not?  Clément, WDYT?
>
> On 2016-12-02 10:07, npostavs@users.sourceforge.net wrote:
>> Okay, let me rephrase.  `python-shell-calculate-command' currently
>> generates a shell command, but none of its callers treat the result as a
>> shell command (they don't pass it to a shell, they parse it with
>> `split-string-and-unquote').  Therefore, the easiest fix is to change
>> `python-shell-calculate-command' to no longer generate a shell command.
>>
>> The other possiblity is to change the callers to treat
>> `python-shell-calculate-command's result as a shell command, but that
>> looks more difficult (though it may be the better solution overall).
>
> Currently, run-python can read a shell command; do we want to remove this feature? If not, then we do need a shell, don't we?

It can "read" a shell command, but won't be able to *run* it unless
it's parseable with split-string-and-unquote, so I don't think we're
removing any feature here.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20744#53

>
> As far as I understand we have two conflicting requirements:
>
> * One part of the code wants access to switches passed to python, as a list of switches.
> * One part of the code wants to read a python command, including switches, from the user.
>
> I'm not sure that we can get these two to both work in all cases, unless we come up with a robust way to parse shell commands given by the user.  I see multiple solutions:
>
> 1. Use a shell to run python. Then the part of the code that wants to know which switches are being passed can use the possibly-incorrect split-string-and-unquote to split user-supplied strings, but the user-supplied command is run as-is through a shell.
>
> 2. Keep running python as a subprocess, without a shell; in that case, user-supplied commands (in C-u M-x run-python) need to be "parsed" back into command + switches before running them, which introduces a small potential for incorrect parsing.
>
> Noam, your approach is (2), right?  I like the simplicity.

Yes, my approach keeps the status quo, it just stops introducing
shell-quoting which could be parsed incorrectly.

>
> In the long run, it would be nice to offer a read-shell-command-as-list function, probably based on eshell.
>
> Cheers,
> Clément.
>

bug#25025: python-shell-calculate-command is wrong

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 258 bytes --]

On 2016-12-02 11:41, Noam Postavsky wrote:
>> Noam, your approach is (2), right?  I like the simplicity.
>
> Yes, my approach keeps the status quo, it just stops introducing
> shell-quoting which could be parsed incorrectly.

Thanks for clarifying!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#25025: python-shell-calculate-command is wrong

[npostavs]

tags 25025 fixed
close 25025 26.1
quit

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Fri, Dec 2, 2016 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> Fine with me (and maybe also change the function's name while you are
>> at it).
>
> If you meant to remove the "shell" from
> `python-shell-calculate-command', I think that refers to the "python
> shell" (which would be called REPL in Lisp speak). There are quite a
> few other functions and variables with the python-shell prefix.

> On Fri, Dec 2, 2016 at 11:15 AM, Clément Pit--Claudel <clement.pit@gmail.com> wrote:
>> 2. Keep running python as a subprocess, without a shell; in that
>> case, user-supplied commands (in C-u M-x run-python) need to be
>> "parsed" back into command + switches before running them, which
>> introduces a small potential for incorrect parsing.
>>
>> Noam, your approach is (2), right?  I like the simplicity.
>
> Yes, my approach keeps the status quo, it just stops introducing
> shell-quoting which could be parsed incorrectly.
>
>>
>> In the long run, it would be nice to offer a read-shell-command-as-list function, probably based on eshell.

Okay, it looks like the consensus is that we should apply the
`combine-and-quote-strings' fix for now, and possibly in future switch
to something more shell based.

I've pushed it to master as 8f611e5e2309 "Fix bad quoting of
python-shell-interpreter", and I'm closing this bug.

bug#25025: python-shell-calculate-command is wrong

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

npostavs@users.sourceforge.net writes:

> Okay, it looks like the consensus is that we should apply the
> `combine-and-quote-strings' fix for now, and possibly in future switch
> to something more shell based.
>
> I've pushed it to master as 8f611e5e2309 "Fix bad quoting of
> python-shell-interpreter", and I'm closing this bug.

This broke the test python-shell-calculate-command-1 on Windows.  We
could update the test to match the new python-shell-calculate-command
implementation, but since the test body is basically just a copy of that
function, it looks pretty useless to me.  I think we should just remove
that test.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1743 bytes --]

From 1173da346a388a7258a8b462ef5a5d9416781245 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Wed, 16 Aug 2017 07:06:38 -0400
Subject: [PATCH] ; Remove python-shell-calculate-command-1 test

* test/lisp/progmodes/python-tests.el
(python-shell-calculate-pythonpath-1): Remove, it merely reprises the
body of `python-shell-calculate-command' and it has been broken on w32
since the fix for Bug#25025 was applied.
---
 test/lisp/progmodes/python-tests.el | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 8795da4ef4..1a8c6a4e8c 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -2546,20 +2546,6 @@ python-tests-shell-interpreter
    (should (string= (python-shell-internal-get-process-name)
                     (format "%s[%s]" python-shell-internal-buffer-name (buffer-name))))))
 
-(ert-deftest python-shell-calculate-command-1 ()
-  "Check the command to execute is calculated correctly.
-Using `python-shell-interpreter' and
-`python-shell-interpreter-args'."
-  (skip-unless (executable-find python-tests-shell-interpreter))
-  (let ((python-shell-interpreter (executable-find
-                                   python-tests-shell-interpreter))
-        (python-shell-interpreter-args "-B"))
-    (should (string=
-             (format "%s %s"
-                     (shell-quote-argument python-shell-interpreter)
-                     python-shell-interpreter-args)
-             (python-shell-calculate-command)))))
-
 (ert-deftest python-shell-calculate-pythonpath-1 ()
   "Test PYTHONPATH calculation."
   (let ((process-environment '("PYTHONPATH=/path0"))
-- 
2.14.1

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 25025@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 16 Aug 2017 07:08:55 -0400
> 
> > Okay, it looks like the consensus is that we should apply the
> > `combine-and-quote-strings' fix for now, and possibly in future switch
> > to something more shell based.
> >
> > I've pushed it to master as 8f611e5e2309 "Fix bad quoting of
> > python-shell-interpreter", and I'm closing this bug.
> 
> This broke the test python-shell-calculate-command-1 on Windows.  We
> could update the test to match the new python-shell-calculate-command
> implementation, but since the test body is basically just a copy of that
> function, it looks pretty useless to me.  I think we should just remove
> that test.

Could you please elaborate about the reason(s) for the breakage of the
test?  I'm not sure I follow, and so cannot make up my mind whether I
agree with removing the test.

Thanks.

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Wed, Aug 16, 2017 at 10:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> Could you please elaborate about the reason(s) for the breakage of the
> test?  I'm not sure I follow, and so cannot make up my mind whether I
> agree with removing the test.
>
> Thanks.

Certainly; this is the test:

(ert-deftest python-shell-calculate-command-1 ()
  "Check the command to execute is calculated correctly.
Using `python-shell-interpreter' and
`python-shell-interpreter-args'."
  (skip-unless (executable-find python-tests-shell-interpreter))
  (let ((python-shell-interpreter (executable-find
                                   python-tests-shell-interpreter))
        (python-shell-interpreter-args "-B"))
    (should (string=
             (format "%s %s"
                     (shell-quote-argument python-shell-interpreter)
                     python-shell-interpreter-args)
             (python-shell-calculate-command)))))

This is the old version of python-shell-calculate-command:

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          (shell-quote-argument python-shell-interpreter)
          python-shell-interpreter-args))

As you can see, the test just repeats the function body and checks for
equality. The new version of python-shell-calculate-command is

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          ;; `python-shell-make-comint' expects to be able to
          ;; `split-string-and-unquote' the result of this function.
          (combine-and-quote-strings (list python-shell-interpreter))
          python-shell-interpreter-args))

Which is no longer the same code as what is in the test. On Unixish
hosts it passes because the default value of
'python-shell-interpreter' "python" doesn't need any escaping to be
shell-quoted. combine-and-quote-strings likewise doesn't do anything
to arguments without spaces, so it gives the same thing. But on w32
shell-quote-argument always puts double quotes around its argument,
causing the test to fail.

We could update the test to use combine-and-quote-strings too, but
just having a second copy of the implementation doesn't seem like a
useful test to me.

bug#25025: python-shell-calculate-command is wrong

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 16 Aug 2017 12:50:25 -0400
> Cc: Clément Pit--Claudel <clement.pit@gmail.com>, 
> 	25025@debbugs.gnu.org
> 
> (ert-deftest python-shell-calculate-command-1 ()
>   "Check the command to execute is calculated correctly.
> Using `python-shell-interpreter' and
> `python-shell-interpreter-args'."
>   (skip-unless (executable-find python-tests-shell-interpreter))
>   (let ((python-shell-interpreter (executable-find
>                                    python-tests-shell-interpreter))
>         (python-shell-interpreter-args "-B"))
>     (should (string=
>              (format "%s %s"
>                      (shell-quote-argument python-shell-interpreter)
>                      python-shell-interpreter-args)
>              (python-shell-calculate-command)))))
> 
> This is the old version of python-shell-calculate-command:
> 
> (defun python-shell-calculate-command ()
>   "Calculate the string used to execute the inferior Python process."
>   (format "%s %s"
>           (shell-quote-argument python-shell-interpreter)
>           python-shell-interpreter-args))
> 
> As you can see, the test just repeats the function body and checks for
> equality.

I see, thanks.  So I guess all it (maybe) tests is that
python-shell-interpreter and what executable-find returns are
identical?  Is that something that could somehow fail and would be
useful to verify?  If not, I agree to removing the test.

bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Wed, Aug 16, 2017 at 12:57 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> I see, thanks.  So I guess all it (maybe) tests is that
> python-shell-interpreter and what executable-find returns are
> identical?  Is that something that could somehow fail and would be
> useful to verify?

No, python-shell-interpreter is by default just "python", not an
absolute filename, so it shouldn't match with executable-find. The
test is let-binding python-shell-interpreter anyway:

    (defvar python-tests-shell-interpreter "python")
    ...
    (ert-deftest python-shell-calculate-command-1 ()
      ...
      (skip-unless (executable-find python-tests-shell-interpreter))
      (let ((python-shell-interpreter (executable-find
                                       python-tests-shell-interpreter))
         ...


>  If not, I agree to removing the test.

Okay, I'll push in a few days.