Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Replace term:// with termopen() to resolve wildcard expansion error #153

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

oddish3
Copy link
Contributor

@oddish3 oddish3 commented Dec 4, 2024

Hi,
This is my first time submitting a PR! 😊

While following along with the Quarto.nvim Kickstarter videos, I encountered the following error when attempting to preview a Quarto document:

E79: Cannot expand wildcards

I thought this was maybe down to my config, so i made a new appname and changed to the nvim-quarto-kickstarter and i got the error again, and i also found this error was reported in Issue #143

Proposed Fix:

I replaced the term:// command invocation with vim.fn.termopen() to prevent the wildcard expansion issue. This change resolves the error and allows the Quarto preview to work as expected.


Behavioral Changes:

  1. The Quarto preview command now successfully runs without triggering the wildcard expansion error.
  2. After running the preview, Neovim switches to the new terminal tab. I'm unsure if this behavior is intended or if it needs further adjustment.

Additional Notes:

Here’s a vid of the current behavior after applying the fix:
video


Testing:

  • Tested on macOS using Fish shell
  • The command now runs without issues
  • Further testing on Linux and Windows would be appreciated to ensure cross-platform compatibility.

Why This Fix Works:

  • term:// can lead to wildcard and quote expansion issues, especially in non-Bash shells like Fish.
  • termopen() directly invokes the command without triggering shell-specific expansions, making it more robust across different environments.

Please let me know if any changes or additional information are needed. Thank you for your time!

@jmbuhr
Copy link
Collaborator

jmbuhr commented Dec 4, 2024

Hi, appreciate the thoughtful PR! I think using termopen is a good idea. I believe the actual fix you introduced is through the use of vim.fn.shellescape in addition to that. But why you did you change (and remove) so much of the other code? Some of the changes I agree with, some just breaks other functionality.

To bring back going back to the quarto buffer from the terminal you need to keep vim.cmd 'tabprevious', or we make it cleaner while we are at it and replace with with a combination of nvim_<get/set>_current_tabpage.

btw. can you provide an example of the kind of filepaths that result in the E79: Cannot expand wildcards error? Would still be nice to reproduce those.

@oddish3
Copy link
Contributor Author

oddish3 commented Dec 4, 2024

Apologies, I hadn't had my coffee yet and I'm still relatively new to lua + programming

I cant pinpoint why i get the error, but i've just included a few print statements in the preview function on a new branch:

[QuartoPreview] Options passed:
  opts: {
  args = "",
  bang = false,
  count = -1,
  fargs = {},
  line1 = 1,
  line2 = 1,
  mods = "",
  name = "QuartoPreview",
  range = 0,
  reg = "",
  smods = {
    browse = false,
    confirm = false,
    emsg_silent = false,
    hide = false,
    horizontal = false,
    keepalt = false,
    keepjumps = false,
    keepmarks = false,
    keeppatterns = false,
    lockmarks = false,
    noautocmd = false,
    noswapfile = false,
    sandbox = false,
    silent = false,
    split = "",
    tab = -1,
    unsilent = false,
    verbose = -1,
    vertical = false
  }
}
  args: ""
[QuartoPreview] Buffer path: /Users/user/vault/test.qmd
[QuartoPreview] Root directory: nil
[QuartoPreview] Platform: Darwin
[QuartoPreview] Non-Windows command constructed
[QuartoPreview] Mode: file
[QuartoPreview] Command to execute: quarto preview '/Users/user/vault/test.qmd'
[QuartoPreview] Detected file extension: .qmd
[QuartoPreview] Attempting to open terminal...
E79: Cannot expand wildcards
[QuartoPreview] Quarto output buffer: 20
[QuartoPreview] Setting up exit autocmd

where as you can see the filepath has no wildcards / none standard characters

My most recent commit has (i think):

  1. added error handling with on_exit callback for termopen
  2. Replaced the original functionality regarding;
  • The file extension checking (quarto_extensions)
  • The mode tracking (mode = 'project' or mode = 'file')
  • The return to previous tab functionality

Hope this helps

@jmbuhr jmbuhr merged commit f810c62 into quarto-dev:main Dec 4, 2024
3 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