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

Expand shell commands #54

Closed
Tracked by #38
superatomic opened this issue Apr 19, 2022 · 5 comments · Fixed by #98
Closed
Tracked by #38

Expand shell commands #54

superatomic opened this issue Apr 19, 2022 · 5 comments · Fixed by #98
Assignees
Labels
deprecation Deprecation of a feature (normally in place of a new one) enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@superatomic
Copy link
Owner

superatomic commented Apr 19, 2022

Part of #38.
We need to specially handle the $(shell command) syntax.

Details:

  • It must translate into the correct form for the given shell. This is easy for Bash, Dash, and Zsh, a bit more complex for Fish, and more challenging for other shells. While we only currently have support for bash, zsh, and fish, we need to make a system that can work for any shell.
  • It must allow for escaping the ) character to be able to use them inside of the expression.
  • You must able be able to escape the $ to not expand it. This is also relevant to Expand shell variables #53.
@superatomic superatomic added enhancement New feature or request help wanted Extra attention is needed on hold Not currently being worked on labels Apr 19, 2022
@superatomic
Copy link
Owner Author

It looks like we need to escape character's ourselves. Instead of worrying about "undoing" the escaping of characters written in a TOML string, we can just use raw TOML strings, which are strings with single quotes (') instead of double quotes (").

We can then write a program to recognize \) as escaping a closing bracket, because \ doesn't escape anything in the TOML representation.

This is a breaking change. We can't really do that much about this. We could try to detect the type of quotations that are used, but that would involve parsing the file and might trigger some false positives (this might be worth it though, if we state that these false positives can happen). Otherwise, the documentation should be changed to use single quotes, and it should be directly stated that using double quotes requires double escaping.

@superatomic superatomic added deprecation Deprecation of a feature (normally in place of a new one) and removed on hold Not currently being worked on labels Apr 25, 2022
@superatomic
Copy link
Owner Author

However, this finally allows us to implement this feature! When this is completed, we can also easily finish the rest of #38.

@superatomic superatomic self-assigned this Apr 25, 2022
@superatomic
Copy link
Owner Author

To be able to issue warnings, we can read through each line of the file and scan for any line that matches /^[A-Za-z0-9\._]+\s*=\s*\[?\s*"/m. If we find any, we will display a warning that recommends switching to single quotes.

@superatomic
Copy link
Owner Author

Also note that the use of double quotes with never be removed, just discouraged for most use cases. This is because after this issue is implemented and merged the new behavior will normally result in having to double escape backslashes while using double quotes.

The warning will either be downgraded it severity or removed in a future version a significant amount of time after the changes in this issue are made. This will probably happen in v1.0.0.

@superatomic superatomic added this to the v0.5.0 milestone May 4, 2022
@superatomic superatomic pinned this issue May 4, 2022
superatomic added a commit to superatomic/dotfiles that referenced this issue May 12, 2022
@superatomic
Copy link
Owner Author

Update: Warnings will not be issued as that could affect actual use cases.

@superatomic superatomic unpinned this issue Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Deprecation of a feature (normally in place of a new one) enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant