-
-
Notifications
You must be signed in to change notification settings - Fork 381
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
Some docs updates #1179
Some docs updates #1179
Conversation
the docs for widgets are automatically generated, but i don't know how magic vars are handled from a glance, it seems to be similar, look at the your checkboxes are missing spaces in order to work properly: some questions i'd like to raise:
|
I don't think you can check whether a script uses a certain environmental variable. So it only exports the Therefore, no slowdowns should be present. Right now there are only three Thank you for the info about the docs. I will mark the pr as draft until I update them |
i mixed up magic constants and variables, my bad. this seems reasonable to me, with little performance overhead then since there's just some strings being thrown around and transformed, compared to the entire application this probably won't make a difference whatsoever. it might be worthwhile to look into updating the docs to mention that the last 3 magic vars are in fact constant |
Well, I tried my best with the docs (I don't have enough willpower to figure out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor wording suggestion, you should be able to simply apply this
apart from that i don't see any issues with this, but i have not tested it.
Co-authored-by: Wölfchen <[email protected]>
one more typo that i just noticed (would be nice if you could just fix that one as well): eww/crates/eww/src/config/inbuilt.rs Line 33 in b3dbfad
Celcius -> degree Celsius
|
thank you <3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while this is a simple export, we should make it useful. EWW_CMD
currently does not work when invoked as one would expect.
i suspect that the cause for this is the two paths in the command being quoted.
the other variables work and can be used as expected.
also, adding this requires eww to remain single-threaded according to the set_env
docs. i don't know whether this could cause issues.
Interesting,
|
i phrased that a bit confusingly. |
Got it, that's kinda weird. Without the quotes it indeed works. Although, when I paste the value with quotes manually, it still works. Don't really understand how it works now, but I will look into it |
So the problem here is that bash interprets quotes literally, as if I was trying to do Two other variables are not quoted since you can quote them and escape possible whitespaces yourself. You can't do that with What I could do is change the format of |
i love bash so much /s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from my admittedly limited testing, this works.
are there issues with this approach?
crates/eww/src/config/inbuilt.rs
Outdated
"EWW_CONFIG_DIR" => DynVal::from_string(eww_paths.get_config_dir().to_string_lossy().into_owned()), | ||
|
||
// @desc EWW_CMD - eww command running in the current configuration, useful in event handlers. I.e.: `:onclick "${EWW_CMD} update foo=bar"` | ||
// @desc EWW_CMD (Magic constant) - eww command running in the current configuration, useful in event handlers. I.e.: `:onclick "${EWW_CMD} update foo=bar"` | ||
"EWW_CMD" => DynVal::from_string( | ||
format!("\"{}\" --config \"{}\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format!("\"{}\" --config \"{}\"", | |
format!("{} --config {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's what I thought, BUT. Try cloning eww into a folder named eww with spaces
for example. It doesn't work for me. The path is just cut from the first space to the end. These are kind of things that make want not to deal with bash ever again(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah true, i forgot about that, nvm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use single quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same problems as with double quotes
Since it seems to be a dead end with the environmental variables, thought I would revert those changes and leave just the docs update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think exporting the two working variables shouldn't do any harm, so we could still export those
but with the current state, i guess this can just be merged
Yeah, it just kinda doesn't feel right. I really doubt that someone uses |
Description
Just some updates smaaall update for docs.
Usage
N/A
Showcase
N/A
Additional Notes
N/A
Checklist
Please make sure you can check all the boxes that apply to this PR.
docs/content/main
directory has been adjusted to reflect my changes.cargo fmt
to automatically format all code before committing