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

%C uses wrong line width when used in description #73

Open
0ion9 opened this issue Sep 15, 2015 · 7 comments
Open

%C uses wrong line width when used in description #73

0ion9 opened this issue Sep 15, 2015 · 7 comments

Comments

@0ion9
Copy link
Collaborator

0ion9 commented Sep 15, 2015

%C appears to use the width of the main item list to determine where to put the text. However, this is larger than the width of the description; therefore, the string being 'centred' is a) not centrally located and b) may be clipped, in the case of longer strings.

How to reproduce:

  • Modify your cmd handler to produce descriptions
    containing strings such as '%C some longish string %C'
  • Exactly what length is needed will depend on your settings, but the point is to select a string that should fit if it were correctly centred.
@tperale
Copy link
Collaborator

tperale commented Sep 15, 2015

Can you post a screenshot ?
Are you running this version of lighthouse #72 with the pango lib (3741041 Makefile didn't work correctly on my machine) ? I the had same issue previously, it was the cairo text api fault, pango lib fixed it.

Also for the length I know the problem and i'm working on it.
Thank you.

@tperale
Copy link
Collaborator

tperale commented Sep 15, 2015

This Makefile works on my machine.

CC=gcc
OBJDIR=objs
SRCDIR=src
INCDIR=$(SRCDIR)/inc
CFLAGS+=-I$(INCDIR)

SRCS=$(wildcard $(SRCDIR)/*.c)
OBJS=$(patsubst $(SRCDIR)/%.c,$(OBJDIR)/%.o,$(SRCS))

CFLAGS+=-O2 -Wall -std=c99
CFLAGS_DEBUG+=-O0 -g3 -Werror -DDEBUG -pedantic
LDFLAGS+=-lxcb -lxcb-xkb -lxcb-xinerama -lxcb-randr -lcairo -lpthread

# OS X keeps xcb in a different spot
platform=$(shell uname)
ifeq ($(platform),Darwin)
    CFLAGS+=-I/usr/X11/include
    LDFLAGS+=-L/usr/X11/lib
endif

# Library specific
HAS_GDK := `pkg-config --exists gdk-2.0 && echo $?`
ifneq $(HAS_GDK,)
    CFLAGS+=`pkg-config --cflags gdk-2.0`
    LDFLAGS+=`pkg-config --libs gdk-2.0`
else
    CFLAGS+=-DNO_GDK
endif
HAS_PANGO :=  `pkg-config --exists pango && echo $?`
ifneq ($(HAS_PANGO),) 
    CFLAGS+=`pkg-config --cflags pango`
    LDFLAGS+=`pkg-config --libs pango`
else
    CFLAGS+=-DNO_PANGO
endif


all: lighthouse

debug: CC+=$(CFLAGS_DEBUG)
debug: lighthouse .FORCE

config: lighthouse .FORCE
    cp -ir ./config/* ~/.config/
    chmod +x ~/.config/lighthouse/cmd*

.FORCE:

lighthouse: $(OBJS)
    $(CC) $(CFLAGS) $^ -o $@ $(LDFLAGS)

$(OBJS): | $(OBJDIR)
$(OBJDIR):
    mkdir -p $@

$(OBJDIR)/%.o: $(SRCDIR)/%.c $(wildcard $(INCDIR)/*.h) Makefile
    $(CC) $(CFLAGS) $< -c -o $@

clean:
    rm -rf $(OBJDIR) lighthouse

@0ion9
Copy link
Collaborator Author

0ion9 commented Sep 16, 2015

I have no idea why you posted that Makefile. This issue is just about the wrong line width -- I have not had any issues with compiling, and I am unsure why you would have gotten the idea that I did have such problems.

Now that you have prompted me to update, I do have some comments though:

  • scripts/basic.py has nonstandard options. Is there any reason not to use --show_execute, --show_in_shell, --term rather than -show-execute, -show-in-shell, -term?
  • The underscores in show_execute and show_in_shell are also nonstandard; unless you have a really good reason, I suggest changing them to dashes which is the GNU standard (eg. show-execute). You will not need to change the code -- the value of an argument to eg. '--show-execute' is stored in the variable show_execute, etc.
  • BTW, if lighthouse is picking up your arguments wrongly and pronouncing them 'invalid option', you should really use --, as in lighthouse -c lighthouse_config -- --show-execute. -- is the standard way with getopt to indicate that getopt should not parse arguments beyond that point. I should have probably clarified the README to demonstrate that point, as it's the best way to pass your extra arguments anyway.
  • The same critique about commandline arguments applies equally to find.py and search.py
  • When I did a git pull to get the new changes, I had to manually resolve conflicts in a lot of files (lighthouse.c, display.c, inc/{results,globals,display}.h, config/lighthouse/{main.py,lighthouserc} . Not sure why that was, it might be my fault TBH.

Trying to compile, I see that NO_PANGO is wrongly being defined. Applying the changes you list above, it seems to correctly detect Pango, but halts with an error :

In file included from src/inc/results.h:7:0, from src/inc/globals.h:7, from src/results.c:12: /usr/include/pango-1.0/pango/pangocairo.h:26:19: fatal error: cairo.h: No such file or directory compilation terminated.

Which, to me, suggests that there is an -I directive that would make cairo.h available that is not being set for some reason (I agree, pkg-config for pango should return that, since it is a dependency). Anyway, not entirely sure why that is happening, I'll update this as things happen.

EDIT: substituting 'pangocairoforpango` in pkg-config calls in the makefile, seems to allow compilation to succeed.

EDIT2: The text in the newly compiled lighthouse appears to be correctly centred, which means this issue is out of date (doesn't relate to git head). Is there any other related issue, or can I close this issue now? From your comments, I wasn't entirely sure if there was another existing related issue.

@0ion9
Copy link
Collaborator Author

0ion9 commented Sep 16, 2015

I've pushed a fix for the missing documentation. I think I got the header size wrong though, the heading 'Passing arguments to cmd' seems too big. Tell me what you think.

@tperale
Copy link
Collaborator

tperale commented Sep 16, 2015

  • I am unsure why you would have gotten the idea that I did have such problems.

Because it's not working for me.

  • The text in the newly compiled lighthouse appears to be correctly centred

Yes the problem came from the cairo cairo_extent function ( 05311e9 ).

  • substituting "pangocairo for pango" in pkg-config calls in the makefile

pango is already used in the pkg-config call, what do you mean ?

  • I've pushed a fix for the missing documentation.

Okay I will change my script.

  • Is there any other related issue, or can I close this issue now?

The only issue is that we have to handle command like this %C%B ... % ...%

@0ion9
Copy link
Collaborator Author

0ion9 commented Sep 16, 2015

pango is already used in the pkg-config call, what do you mean ?
That using 'pango' in the pkg-config call does not work -- it generates the error I described, /usr/include/pango-1.0/pango/pangocairo.h:26:19: fatal error: cairo.h: No such file or directory compilation terminated.

Changing 'pango' into 'pangocairo' allows compilation to succeed, for me.

Sorry, there was probably a little confusion about what happened with the Makefile.

  • Once I updated to latest git version, Pango was not being detected.
  • Then, I added your changes that you pasted above.
  • At that point, the compilation error I talked about appeared.
  • I made these two changes:
    • your line CFLAGS+=\pkg-config --cflags pangobecame`CFLAGS+=`pkg-config --cflags pangocairo
    • your line LDFLAGS+=\pkg-config --libs pangobecame`LDFLAGS+=`pkg-config --libs pangocairo
  • After making those changes, I was able to successfully compile and run lighthouse.

The only issue is that we have to handle command like this %C%B ... % ...%

I see, the first % is interpreted (correctly) as closing the 'bold' section, but also is interpreted (wrongly) as closing the 'centred' section.
Well, that's what it seems like to me, when I try it.

@tperale
Copy link
Collaborator

tperale commented Sep 27, 2015

#76 & #77

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

No branches or pull requests

2 participants