rjkroege / edwood

Go version of Plan9 Acme Editor
Other
380 stars 34 forks source link

Spaces in names #483

Closed paul-lalonde closed 1 year ago

paul-lalonde commented 1 year ago

Get on a directory now wraps filenames with spaces with single quotes. Tag of a file with a space in the name now has the filename surrounded by single quotes. Look now tries to find a file between single quotes, and correctly extends to an address suffix: 'a b':/foo

codecov[bot] commented 1 year ago

Codecov Report

Merging #483 (148696f) into master (dd8fcfc) will increase coverage by 0.24%. The diff coverage is 95.16%.

:exclamation: Current head 148696f differs from pull request most recent head cef9cec. Consider uploading reports for the commit cef9cec to get more accurate results

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   58.09%   58.34%   +0.24%     
==========================================
  Files          53       53              
  Lines       10676    10754      +78     
==========================================
+ Hits         6202     6274      +72     
- Misses       4040     4043       +3     
- Partials      434      437       +3     
Impacted Files Coverage Δ
look.go 56.83% <93.87%> (+5.20%) :arrow_up:
exec.go 38.06% <100.00%> (+0.09%) :arrow_up:
tagindex.go 85.52% <100.00%> (+0.39%) :arrow_up:
text.go 48.89% <100.00%> (+0.54%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rjkroege commented 1 year ago

The windows failures seem real: the logic for determining when to use the ' character is (I think) treating all paths on Windows as requiring the '. Is that the correct semantic? It breaks a bunch of tests. 🙂

Also: do you want me to fix? I'm sure that GCE has changed how I go about spinning up a Windows VM so some delay would be entailed.

paul-lalonde commented 1 year ago

I can likely fix it from the airport this afternoon, so long as I can keep using the GitHub runner for the windows tests.

On Sun, Apr 23, 2023, 11:23 a.m. Robert Kroeger @.***> wrote:

The windows failures seem real: the logic for determining when to use the ' character is (I think) treating all paths on Windows as requiring the '. Is that the correct semantic? It breaks a bunch of tests. 🙂

Also: do you want me to fix? I'm sure that GCE has changed how I go about spinning up a Windows VM so some delay would be entailed.

— Reply to this email directly, view it on GitHub https://github.com/rjkroege/edwood/pull/483#issuecomment-1519092196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6RUGYMOUOG3IZIP2PAMLXCVCOLANCNFSM6AAAAAAXIQSMVE . You are receiving this because you authored the thread.Message ID: @.***>

paul-lalonde commented 1 year ago

This (mostly) passing status is a lie. I have some new tests to commit, and a larger refactor incoming for expandfile().

paul-lalonde commented 1 year ago

There is a remaining bug: Look-clicking in an address following a quoted filename fails. The context has to be extended to find a quote before a possible colon.

paul-lalonde commented 1 year ago

There remains some "tricky bits", though this is now as functional as acme would be if it had spaces-in-names handling. The edge cases surround ugly things like colons in filenames which basically break all acme's filename handling. This work is all made harder by acme's great avoidance of ever copying data out of text buffers. We've already started relaxing that in places and filename handling seems like one place it would be worthwhile. The Expand struct could start holding strings for filename and address instead of the q0,q1 a0,a1 offsets, for example, which would allow much easier handling of escape quoting.
But for a day's work, I think this is ready. I'm now using it, and not having too bad a time with spaces anymore.

paul-lalonde commented 1 year ago

All tests pass. Look is broken. Do not commit code while in a daze in an airport. Thankfully I didn't merge.