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

Highlighting code #5

Closed
davmaz opened this issue Aug 19, 2016 · 18 comments
Closed

Highlighting code #5

davmaz opened this issue Aug 19, 2016 · 18 comments

Comments

@davmaz
Copy link

davmaz commented Aug 19, 2016

Is it possible to use the highlight program to highlight go code when you display it? This would be a really cool enhancement since most users are gophers.

Sorry for the request when it's not really an issue. Regardless, I'm addicted to "lf" already -- great work!

@gokcehan
Copy link
Owner

Issues are fine for feature requests. We don't have anything else for discussion anyway.

You mean syntax highlight in the preview pane? Currently it is not possible to do that but it may change in the future. There are a couple of issues that needs to be addressed first though. Your best option for now may be to use a syntax highlighting pager if you're not using any yet.

@davmaz
Copy link
Author

davmaz commented Aug 21, 2016

Yes, syntax highlighting in the preview pane. I wondered if the well known 'highlight' program, if installed on the host, could be used to do the work. That way you wouldn't have to bother with how it's done, but I need a way to "inline" this with the text going to the preview pane. Maybe something that could be configured/handled by a lfrc macro?

@gokcehan
Copy link
Owner

I think that would be a nice addition. I will try to take a look at it in a few hours. Couple of points that worry me though:

  • I'm not sure how ansi escape codes will interact with termbox. We are gonna need to try and see.
  • Since the highlighter program will be called for every file it shouldn't have a long startup time. I was worried about this when I thought pygments but highlight doesn't seem to have such a problem (first time I heard about it by the way).
  • We need to decide a proper abstraction level for previewing files. Since lf doesn't know about source code files, a more general previewing method could be more appropriate. Users can check the extension and/or mimetype to decide if syntax highlighting should be done. This would also allow other things such as previewing pdf files or even maybe inline images. I think this is the approach ranger is using. I will check how they do it.
  • Currently lf only reads the first n lines of a file for preview, n being the number of rows in the terminal. External programs should also have a way to do this. It is mostly a non-issue for small files such as source code and such but it can be important for bigger files such as those having multiple GBs of data in them.

@gokcehan
Copy link
Owner

I have tried an example with ansi escape codes from highlight and it seems that termbox does not play well with it. I have been looking at the termbox source for a while but there doesn't seem to be a way to manually handle ansi escape codes with the current API based on Cell. So at this point I can think of two solutions. First we can scan the input and convert the escape codes to proper termbox colors/attributes which may require a significant amount of work, or we can try to manually move and directly print to stdout which will most likely introduce some bugs. I will try the latter approach when I have some time.

@KenjiTakahashi
Copy link
Contributor

I planned to write about this as well, but got away on weekend and I see things unrolled without me anyway.

The way I thought about it, I'd let user set any command as "preview preprocessor" and call it with file type (mime-type?) as first argument, then pipe in the file contents to stdin. That way, user can do pretty much what he wants*, but we remain in control to, e.g. send only part of the whole file, if we choose so.

The ranger abstraction is AFAIK a bit "lower", i.e. there are certain apps assigned to previewing certain filetypes. For example, if you have highlight installed, it will be used to highlight code, but last I checked there was no way to configure a different highlighter (might have changed, though).

*) I'm afraid image previews might not work that way, though. Aside from macOS's iTerm, I don't think there are any terminals that have image display support. Using w3img (the thing that ranger uses on Linux) is pretty much all a big hack ;-).

Just my two cents.

@gokcehan
Copy link
Owner

I'm thinking the same thing about the preview processor @KenjiTakahashi but it's important to make ansi escape codes work first, at least for the sake of this issue. The rest should be straightforward. I have checked ranger's scope.sh and it seems that they are using file utility to determine the mimetype and then use either highlight or pygmentize for highlighting. Output format is either ansi or xterm256 which are both based on escape codes. I'm guessing ncurses doesn't have any issues handling those.

Yesterday I have tried to manually move the cursor using ansi codes and print directly to the preview pane without termbox and it was kind of a mess as I thought it would be. I think the only viable option we have is to do this with termbox. I have been reading the termbox source today and found the related section here. It looks like termbox converts all characters less than ' ' (ascii 32) to ' ' including the escape character (ascii 27). Adding an exception for escape key makes the colors work but then the positions are messed. I guess termbox is simply not designed to work like that.

Meanwhile, I'm getting less convinced that this is an essential feature. Ranger needs to do this since it is using the same thing for it's preview pane and built-in pager. In lf pager highlighting can be done easily with something like map p $highlight --out-format=ansi $f | less -R and I'm not sure if it's of much use to have syntax highlighting in the preview pane. Personally I got so used to calling the pager when I start reading anything other than maybe header comments in source files.

gokcehan added a commit that referenced this issue Aug 28, 2016
gokcehan added a commit that referenced this issue Aug 28, 2016
@gokcehan
Copy link
Owner

Ok, so I have pushed a commit to evaluate ansi codes while printing. To be honest it was easier than imagined. It turned out to be a clean and reasonable solution that we can maintain. This makes it possible to view colors in a file when they are coded with ansi codes. You can try this with something like highlight --out-format=ansi foo.go > out.txt and then preview out.txt in lf.

Second commit is to do this automatically by setting an executable with previewer option. I was thinking of making this a command just like open-file so that it would be possible to write a custom script right inside the configuration file but it looks difficult with the current structure of the code. Plus we should only allow $ commands for file preview. Also people are likely to use the same filter for both preview and pager and without an external script I'm not sure how to do that without duplication in the configuration. So maybe this is better for now.

I tried this with ranger's scope.sh and I can feel some lag moving between files. Since this is an optional feature I think it is ok for now. We may try to optimize it if possible when we start profiling the code. I suspect though this is probably because ranger is doing the preview asynchronously.

I haven't updated the documentation yet as we still need some polish. I'm leaving this issue open for now.

@davmaz
Copy link
Author

davmaz commented Sep 3, 2016

Hello,
Thanks for the information. I will try what you suggested. I REALLY love lf. I'm a touch typist, and every time I have to move my hand off the 'home' keys (to the mouse, or an arrow key) it bothers me. I have made my editor (sublime text 3) the default editor if I hit an 'l' (el).
To me, this is one of the best go programs I've seen. Unfortunately I can't star your page more than once -- or I would!

Best,
Dave


From: gokcehan [notifications@github.com]
Sent: Sunday, August 28, 2016 08:24
To: gokcehan/lf
Cc: davem@ltsnet.net; Author
Subject: Re: [gokcehan/lf] Highlighting code (#5)

Ok, so I have pushed a commit to evaluate ansi codes while printing. To be honest it was easier than imagined. It turned out to be a clean and reasonable solution that we can maintain. This makes it possible to view colors in a file when they are coded with ansi codes. You can try this with something like highlight --out-format=ansi foo.go > out.txt and then preview out.txt in lf.

Second commit is to do this automatically by setting an executable with previewer option. I was thinking of making this a command just like open-file so that it would be possible to write a custom script right inside the configuration file but it looks difficult with the current structure of the code. Plus we should only allow $ commands for file preview. Also people are likely to use the same filter for both preview and pager and without an external script I'm not sure how to do that without duplication in the configuration. So maybe this is better for now.

I tried this with ranger's scope.shhttps://github.com/ranger/ranger/blob/master/ranger/data/scope.sh and I can feel some lag moving between files. Since this is an optional feature I think it is ok for now. We may try to optimize it if possible when we start profiling the code. I suspect though this is probably because ranger is doing the preview asynchronously.

I haven't updated the documentation yet as we still need some polish. I'm leaving this issue open for now.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/5#issuecomment-242971971, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAgSBtk2NozwJhH7K3IKKE3JKNP8-4Avks5qkX4OgaJpZM4JomvF.

@davmaz
Copy link
Author

davmaz commented Sep 3, 2016

Hello again,
Is there a 'toggle' command? In my lfrc, I've mapped 'sh' to hidden. It would be great (for me that is) to be able to use the same command again to turn off hidden. Alternatively, should I map 'sh' to a script that can keep track of whether hidden files are shown or not and toggle the feature?

Best,
Dave


From: gokcehan [notifications@github.com]
Sent: Sunday, August 28, 2016 08:24
To: gokcehan/lf
Cc: davem@ltsnet.net; Author
Subject: Re: [gokcehan/lf] Highlighting code (#5)

Ok, so I have pushed a commit to evaluate ansi codes while printing. To be honest it was easier than imagined. It turned out to be a clean and reasonable solution that we can maintain. This makes it possible to view colors in a file when they are coded with ansi codes. You can try this with something like highlight --out-format=ansi foo.go > out.txt and then preview out.txt in lf.

Second commit is to do this automatically by setting an executable with previewer option. I was thinking of making this a command just like open-file so that it would be possible to write a custom script right inside the configuration file but it looks difficult with the current structure of the code. Plus we should only allow $ commands for file preview. Also people are likely to use the same filter for both preview and pager and without an external script I'm not sure how to do that without duplication in the configuration. So maybe this is better for now.

I tried this with ranger's scope.shhttps://github.com/ranger/ranger/blob/master/ranger/data/scope.sh and I can feel some lag moving between files. Since this is an optional feature I think it is ok for now. We may try to optimize it if possible when we start profiling the code. I suspect though this is probably because ranger is doing the preview asynchronously.

I haven't updated the documentation yet as we still need some polish. I'm leaving this issue open for now.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/5#issuecomment-242971971, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAgSBtk2NozwJhH7K3IKKE3JKNP8-4Avks5qkX4OgaJpZM4JomvF.

@davmaz
Copy link
Author

davmaz commented Sep 4, 2016

Hey,
Sorry for not reading the tutorial. You DO already have the toggle ability with !. Very nice.

Best,
Dave


From: gokcehan [notifications@github.com]
Sent: Sunday, August 28, 2016 08:24
To: gokcehan/lf
Cc: davem@ltsnet.net; Author
Subject: Re: [gokcehan/lf] Highlighting code (#5)

Ok, so I have pushed a commit to evaluate ansi codes while printing. To be honest it was easier than imagined. It turned out to be a clean and reasonable solution that we can maintain. This makes it possible to view colors in a file when they are coded with ansi codes. You can try this with something like highlight --out-format=ansi foo.go > out.txt and then preview out.txt in lf.

Second commit is to do this automatically by setting an executable with previewer option. I was thinking of making this a command just like open-file so that it would be possible to write a custom script right inside the configuration file but it looks difficult with the current structure of the code. Plus we should only allow $ commands for file preview. Also people are likely to use the same filter for both preview and pager and without an external script I'm not sure how to do that without duplication in the configuration. So maybe this is better for now.

I tried this with ranger's scope.shhttps://github.com/ranger/ranger/blob/master/ranger/data/scope.sh and I can feel some lag moving between files. Since this is an optional feature I think it is ok for now. We may try to optimize it if possible when we start profiling the code. I suspect though this is probably because ranger is doing the preview asynchronously.

I haven't updated the documentation yet as we still need some polish. I'm leaving this issue open for now.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/5#issuecomment-242971971, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAgSBtk2NozwJhH7K3IKKE3JKNP8-4Avks5qkX4OgaJpZM4JomvF.

@KenjiTakahashi
Copy link
Contributor

@davmaz Please keep issues on topic, when possible, so others do not get spammed with unrelated notifications. Thanks :-).

@gokcehan
Copy link
Owner

gokcehan commented Sep 4, 2016

@KenjiTakahashi @davmaz I have now added a google groups mailing list and a gitter chat (seems to support IRC as well) for questions and discussions. You can find the links in the readme if you are interested.

gokcehan added a commit that referenced this issue Sep 15, 2016
gokcehan added a commit that referenced this issue Sep 15, 2016
gokcehan added a commit that referenced this issue Sep 15, 2016
gokcehan added a commit that referenced this issue Sep 15, 2016
@gokcehan
Copy link
Owner

I have now added file previewing to the documentation. Now the only thing left to do is to call the preview script asynchronously. I have tried something but couldn't get my head around synchronization so I leave it as it is for now.

@KenjiTakahashi
Copy link
Contributor

By calling cmd with .Start it is already called asynchronously, I don't think there's anything more to do in that regard. The only thing I can think of is to call Wait in a goroutine at the end (it must be called after we've read all that we wanted from the pipe) to make sure that cmd resources are freed once the command finishes.

@KenjiTakahashi
Copy link
Contributor

Hm, sth's not right. My previewer stopped working after the last update. Will check why tomorrow, no time now.

@gokcehan
Copy link
Owner

@KenjiTakahashi Ahh, it's leaking without Wait right? Fixing it right now.

It's probably not working because I have removed width parameter as it wasn't immediately available where I moved the code. I can add it back but as you said it is not very useful anyways.

gokcehan added a commit that referenced this issue Sep 16, 2016
gokcehan added a commit that referenced this issue Sep 16, 2016
@KenjiTakahashi
Copy link
Contributor

KenjiTakahashi commented Sep 16, 2016

It's probably not working because I have removed width parameter as it wasn't immediately available where I moved the code. I can add it back but as you said it is not very useful anyways.

Right! Works again now. I don't care about width, either, so I think we can leave it the way it is now. Maybe it will be useful when I get to displaying images, but I don't know yet, it's not very easy :-).

gokcehan added a commit that referenced this issue Jan 29, 2018
gokcehan added a commit that referenced this issue Feb 10, 2018
@gokcehan
Copy link
Owner

Asynchronous previewing is implemented while working on #92. I'm closing this issue now. If there is anything left to do or there are some regressions we can track it in the other issue. Particularly we could store the modification date of the file during preview and invalidate the value in the cache if the current modification date is more recent. I need to see if need an extra stat call for this purpose.

P.S It's always encouraging to see an 18+ month-old issue going down :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants