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

Copy to clipboard #590

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Copy to clipboard #590

wants to merge 12 commits into from

Conversation

rben01
Copy link
Contributor

@rben01 rben01 commented Sep 29, 2024

This is copy-to-clipboard functionality that was mentioned in #394. The copy copies the last result to the clipboard, formatted according to the user’s config or falling back to Numbat’s default display if no config, and prints that it has done so. In general I try to fall back to Numbat’s own defaults with user configuration where it makes sense. Here is the default config for this:

[copy-output-config.int-config]
grouping = "standard"
separator = ","
minus-sign = "-"

[copy-output-config.float-config]
max-sig-digits = 0
min-sig-digits = 0
max-width = 0
decimal = "."
capitalize-e = false

[copy-output-config.unit-config]
fancy-exponents = false
multiplication-operator = "·"
division-operator = "/"
space-btwn-operators = false

Please bike-shed away on these configuration options — names, default values, anything extraneous or missing...

With the above config (generated by numbat --generate-config), you'd get this kind of output:

>>> copy  # first command in the session
error: no value to copy
>>> 1e12

  1.0e+12

    = 1.0e+12

>>> copy
1,000,000,000,000
>>> 1kg m^2/s^2

  1 kilogram × metre² / second²

    = 1 kg·m²/s²    [Energy or Torque]

>>> copy
1 kg·m^2/s^2
>>> 

@rben01 rben01 changed the title Copy to clipboard (not complete as of submission) Copy to clipboard Sep 30, 2024
@rben01
Copy link
Contributor Author

rben01 commented Sep 30, 2024

This works only for the CLI frontend. I need to think about how to enable similar functionality for other frontends, ideally without reimplementing it from scratch — hopefully the only thing to be reimplemented would be the clipboard copying itself. Related to #584

@sharkdp
Copy link
Owner

sharkdp commented Oct 8, 2024

Thank you very much. This does not seem to work for me on Linux (using a standard X server with i3). After typing copy, I get the expected message:

>>> 2+3

  2 + 3

    = 5

>>> copy
5 was copied to the clipboard

but the 5 is nowhere to be found, neither in the "ctrl+v" clipboard (X11: clipboard / secondary), nor in the "middleclick" clipboard (X11: primary).

@rben01
Copy link
Contributor Author

rben01 commented Oct 9, 2024

Hmm, how about now? (I don't have a linux box to test with.) Seems like linux does some weird stuff where dropping the clipboard clears its contents. Also it looks like when numbat-cli exits, the clipboard’s contents will be cleared for the same reason. Is it worth working around this? The arboard docs show how to do this, but it involves spawning a background process to host the clipboard, which seems like a lot of complexity for a small gain.

@sharkdp
Copy link
Owner

sharkdp commented Oct 9, 2024

Hmm, how about now? (I don't have a linux box to test with.)

Now I get an error message (same as in CI: https://github.com/sharkdp/numbat/actions/runs/11258512679/job/31305297678?pr=590#step:8:258), but if I fix that as suggested, I can compile it.

When I try to copy something by typing copy, numbat hangs without any output:

>>> 1

  1

    = 1

>>> copy

Interestingly, as soon as I copy something into the clipboard (anything), the process proceeds and prints

1 was copied to the clipboard

But nothing is in fact copied.

@rben01
Copy link
Contributor Author

rben01 commented Oct 9, 2024

Oh, so wait literally waits. I'm guessing then that using the non-Linux code for Linux should work, at least as long as numbat is running?

@sharkdp
Copy link
Owner

sharkdp commented Oct 10, 2024

I'm guessing then that using the non-Linux code for Linux should work, at least as long as numbat is running?

You mean the state you had before? (c7b4f89)

No. It doesn't work. Even if numbat is still open.

Also it looks like when numbat-cli exits, the clipboard’s contents will be cleared for the same reason. Is it worth working around this?

I don't think so. A couple of applications have a similar behavior. It's slightly annoying, but I don't think it's a big problem for Numbat.

@rben01
Copy link
Contributor Author

rben01 commented Oct 10, 2024

How about now?

@sharkdp
Copy link
Owner

sharkdp commented Oct 10, 2024

Now it works — thank you!

I will do a full review shortly.

I will already note that I am a bit overwhelmed by the number of configuration options. I'm usually trying to keep the number of options very small, unless it's definitely necessary to add them. I'm not (yet) very strict about being backwards compatible, but once we are, we need to maintain all of those options.

@rben01
Copy link
Contributor Author

rben01 commented Oct 10, 2024

I will already note that I am a bit overwhelmed by the number of configuration options.

That's fair. I naively added (nearly) as much as was supported, with almost no editing. max-sig-digits, min-sig-digits, and max-width can probably all go. The rest I feel are important to keep, though.

@sharkdp
Copy link
Owner

sharkdp commented Oct 23, 2024

max-sig-digits, min-sig-digits, and max-width can probably all go. The rest I feel are important to keep, though.

If it's not too much trouble, it would be great if we could remove them for now. Thank you.

@rben01
Copy link
Contributor Author

rben01 commented Oct 24, 2024

Actually very little trouble! I also renamed copy_output_config ([copy-output-config] in TOML) to copy_result, as the docs refer to the outcome of a calculation as the result, and “config” seemed redundant in that case. (I left int-config, float-config, unit-config as is.)

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