Skip to content

Conversation

@mugraph
Copy link
Contributor

@mugraph mugraph commented May 7, 2025

❤️ Thank you for your contribution!

Description

This PR makes sure the instance path is updated correctly, even if the user has editing_mode='vi' set in their ipython config, so far resulting in funky instance paths like .virtualenvs/mysite/var/instance$'\033'\[0\ q, effectively breaking assets build.

Related issue: #361

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Frontend

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

* explicitly unset ipython editing_mode for printing instance path,
  as ipython appends ansi escape sequence in vim mode that would lead to
  instance paths like `.virtualenvs/mysite/var/instance$'\033'\[0\ q`,
  effectively breaking the assets build

closes: inveniosoftware#361
Copy link
Contributor

@max-moser max-moser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

i quickly checked the other spots where invenio shell would be called (rg 'invenio.*\n?.*shell' -UC3 invenio_cli), and this seems to be the only place where we care about the command's output (beyond printing it out).

in the long run, i think @scossu's idea to use a simpler python interpreter (python over ipython) might be a broader fix.
however, this will do for the meanwhile.

@max-moser max-moser merged commit c7086bd into inveniosoftware:master May 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants