Skip to content

Add support for evaluating Vim expressions in a heredoc#10138

Closed
yegappan wants to merge 1 commit intovim:masterfrom
yegappan:cmdexpand
Closed

Add support for evaluating Vim expressions in a heredoc#10138
yegappan wants to merge 1 commit intovim:masterfrom
yegappan:cmdexpand

Conversation

@yegappan
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Apr 10, 2022

Codecov Report

Merging #10138 (50fd646) into master (34ffa10) will increase coverage by 2.72%.
The diff coverage is 98.75%.

❗ Current head 50fd646 differs from pull request most recent head 3d84a66. Consider uploading reports for the commit 3d84a66 to get more accurate results

@@            Coverage Diff             @@
##           master   #10138      +/-   ##
==========================================
+ Coverage   80.95%   83.67%   +2.72%     
==========================================
  Files         161      154       -7     
  Lines      185383   176636    -8747     
  Branches    41905    39726    -2179     
==========================================
- Hits       150076   147806    -2270     
+ Misses      22765    16789    -5976     
+ Partials    12542    12041     -501     
Flag Coverage Δ
huge-clang-none 82.40% <96.25%> (-0.01%) ⬇️
huge-gcc-none 82.74% <98.71%> (?)
huge-gcc-unittests 2.00% <0.00%> (?)
linux 83.67% <98.75%> (+1.26%) ⬆️
mingw-x64-HUGE ?
mingw-x64-HUGE-gui ?
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/evalvars.c 91.01% <98.57%> (-0.25%) ⬇️
src/userfunc.c 89.22% <100.00%> (-0.38%) ⬇️
src/libvterm/src/unicode.c 70.83% <0.00%> (-13.17%) ⬇️
src/if_perl.xs 77.32% <0.00%> (-4.24%) ⬇️
src/highlight.c 78.66% <0.00%> (-2.59%) ⬇️
src/misc2.c 87.21% <0.00%> (-1.78%) ⬇️
src/time.c 87.61% <0.00%> (-1.71%) ⬇️
src/arabic.c 93.47% <0.00%> (-1.65%) ⬇️
src/sha256.c 94.89% <0.00%> (-1.55%) ⬇️
src/buffer.c 84.86% <0.00%> (-1.47%) ⬇️
... and 150 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34ffa10...3d84a66. Read the comment docs.

@yegappan yegappan force-pushed the cmdexpand branch 4 times, most recently from c604d73 to 3d5bcb3 Compare April 10, 2022 15:03
@yegappan yegappan changed the title WIP: Add support for evaluating expression in a heredoc Add support for evaluating expression in a heredoc Apr 10, 2022
@yegappan yegappan changed the title Add support for evaluating expression in a heredoc Add support for evaluating Vim expressions in a heredoc Apr 10, 2022
@brammool
Copy link
Contributor

Let's leave this open a few days to allow for comments.

I'm not sure if we should use =expr or just expr. Compared to other places in Vim =expr would be more common,
but compared to any other language expr would be preferred.

The help is not clear: can the expression span multiple lines? So I could use

some literal text `=
 expr line
  expr line
` more literal text

@vim-ml
Copy link

vim-ml commented Apr 10, 2022 via email

@yegappan yegappan force-pushed the cmdexpand branch 3 times, most recently from 8838a23 to 806fa19 Compare April 11, 2022 02:33
@lacygoill
Copy link

I'm not sure if we should use =expr or just expr. Compared to other places in Vim =expr would be more common,
but compared to any other language expr would be preferred.

Yes, I suggested backtick equal to be as consistent as possible with the existing syntax, but in practice I suspect it's hardly ever used. Dropping the equal sign might make it easier for people coming from other languages to assimilate this new syntax.

@vim-ml
Copy link

vim-ml commented Apr 11, 2022 via email

@vim-ml
Copy link

vim-ml commented Apr 11, 2022 via email

@yegappan yegappan force-pushed the cmdexpand branch 2 times, most recently from 98ea5a8 to 6b707d2 Compare April 13, 2022 03:47
@brammool
Copy link
Contributor

The help says "If the expression evaluation fails, then it is replaced with an empty string."
I think it's better to treat an error like an error and not silently use an empty string.
If you would have an assignment with a faling expression, it would also fail.

@yegappan
Copy link
Member Author

The help says "If the expression evaluation fails, then it is replaced with an empty string."
I think it's better to treat an error like an error and not silently use an empty string.
If you would have an assignment with a faling expression, it would also fail.

I have updated the PR to fail the assignment if an expression evaluation fails.

@yegappan yegappan force-pushed the cmdexpand branch 6 times, most recently from a43a70f to 5e57b51 Compare April 16, 2022 03:53
@brammool
Copy link
Contributor

I have not seen comments objecting to using the "=expr" form, including the equal sign. So I guess it should be OK that way.

@brammool
Copy link
Contributor

When adding another test I found that this doesn't work:

var x = 'X'
  code =<< eval trim END
    let b = `=x`
  END

This will require more work, since "x" would have to be evaluated at runtime. Let's do that in a followup change.

@brammool brammool closed this in efbfa86 Apr 17, 2022
@vim-ml
Copy link

vim-ml commented Apr 17, 2022 via email

@errael
Copy link
Contributor

errael commented Apr 17, 2022

I've never use vimscript, or the = that I see around occasionally. From my perspective, whatever is consistent with what I'll see in vim9script is best; :help script explains vim9. Consistency with vim9 seems paramount. IMO, if the = is required in vim9 in situations, then OK; otherwise nuke it.

@Shane-XB-Qian

This comment was marked as off-topic.

@Shane-XB-Qian

This comment was marked as off-topic.

@brammool
Copy link
Contributor

@cecamp for the highlighting

zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Apr 15, 2023
Problem:    Cannot easily mix expression and heredoc.
Solution:   Support  in heredoc. (Yegappan Lakshmanan, closes vim/vim#10138)

vim/vim@efbfa86

Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
folke pushed a commit to folke/neovim that referenced this pull request May 22, 2023
Problem:    Cannot easily mix expression and heredoc.
Solution:   Support  in heredoc. (Yegappan Lakshmanan, closes vim/vim#10138)

vim/vim@efbfa86

Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
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.

6 participants