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

add simple launcher menu entry #1

Merged
merged 1 commit into from
Mar 20, 2018
Merged

add simple launcher menu entry #1

merged 1 commit into from
Mar 20, 2018

Conversation

i4ki
Copy link

@i4ki i4ki commented Mar 20, 2018

No description provided.

Signed-off-by: i4k <tiago4orion@gmail.com>
@i4ki i4ki requested a review from katcipis March 20, 2018 19:44
Copy link
Member

@katcipis katcipis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -205,15 +238,9 @@ spawn(ScreenInfo *s)
signal(SIGINT, SIG_DFL);
signal(SIGTERM, SIG_DFL);
signal(SIGHUP, SIG_DFL);
if(termprog != NULL){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea to extract this logic from here and inject a function =D

void
runterm() {
if(termprog != NULL){
execl(shell, shell, "-c", termprog, (char*)0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the "-c" option specific to some shells ? (I remember it vaguely from bash)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-c is specific to all posix compliant shells...

http://pubs.opengroup.org/onlinepubs/009695399/utilities/sh.html

AFAIK, the shells below have this option:
sh, rc, bash, csh, dash, fish, zsh, nash

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if you recite the sequence of shell names above very fast you get brain damage kkkk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auheauheauheuaheuhauheuahe

Did not know that, thanks =)

@katcipis katcipis merged commit 4fef57c into master Mar 20, 2018
katcipis pushed a commit that referenced this pull request Jan 22, 2022
Fixes 9fans#122, 9fans#140.

As reported in 9fans#122, `file:1:1` moves to the end of the file,
and `file:1:2` fails with “address out of range”.

I’ll use file:2:3 as an example so we can tell the line and column number apart.

What’s happening is this:
plumb/basic matches `2:3` using twocolonaddr (from plumb/fileaddr),
then sets addr to `2-#1+#3`
(the 1 is constant and was introduced because column numbers are 1-based).
Acme interprets this in three steps:

1. find the range (q0, q1) that contains line 2
2. create the range (q2, q2) where q2 = q0 - 1
3. create the range (q3, q3) where q3 = q2 + 3

The second step has a branch where if q0 == 0 and 1 > 0
(remember that 1 is constant and comes form plumb/basic),
q0 is set to the end of the file.
This makes addressing things at the end of the file easier.

The problem then is that if we select line 1,
which starts at the beginning of the file,
q0 is always 0 and the branch in step 2) will always be used.
`1:1` is interpreted as `1-#1+#1` which starts at 0, wraps around to the end of the file, then moves 1 character backwards and then forwards again, ending at the end of the file.
`1:2` is interpretes as `1-#1+#2` which starts at 0, wraps around to the end od the file, then moves 1 character backwards and tries moving 2 characters forwards beyond the end of the file, resulting in the out of range error.

In 9fans#140 @rsc proposed transforming `:X:Y` into `:X-#0+#Y-#1` instead since that
avoids wrapping around by not moving backwards at first.
This change modifies `plumb/basic` to do that.
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